Should I assume data passed to my function is accurate?

Question

Assuming I have the following struct (just an example)

struct string{
    int len;
    char*str;
}

And I have the function

int init_str(struct string*s, int len);

which will perform

s->len=len;
s->str=malloc(len*sizeof(char))

And then I have the function

int do_something(struct string*s, ...)

which will assume s->str is a validly malloc()'ed pointer

Should I assume that s->str is a malloc()'ed pointer set by init_str()?

Answer 1

If there is any way to be sure that the struct is valid, then go ahead and assume it is valid.

However, if you want the application to be robust, you should assume nothing - and just check if everything is OK.

Note: one way to ensure that the structure is valid is to know that another function is called before "do_something()" and that function validates everything. Even so, it is dangerous, because that other function might change in the future, and remove those checks - leaving your function to operate on non-validated data.

Note: the discussion might continue based on how you plan to use the data in the structure, but that is pretty much out of the scope of the question.

Note: from an extreme POV, the structure is always valid. You have a starting address and a length, and therefore you have a "string" (actually a buffer). If that string is relevant to you, or if you should (or should not) modify that string, is another discussion :)

Note 2: as a matter of fact, you cannot really validate a pointer. The only invalid pointer is NULL, but even then things can get tricky. I used in the past a microcontroller, and port A was physically mapped to address 0x00. So reading port A could actually done by the most wrong thing to do in C: de-referencing the NULL pointer. We had to hack the results of the static analysis tool to filter out the statements involving port A, before the remaining results became readable.

---

Should I assume that s->str is a malloc()ed pointer set by init_str()?

If you read the documentation properly, you will see that even "malloc()" itself can fail - and in that case it will return error codes. So even if you make sure that "malloc()" is called, you cannot know at the time of writing the software if the call to "malloc()" wil succeed or if it will fail. So verifying the return value of "malloc()" and designing the subsequent code based on the returned value is the safe way to go.

Answer 2

If you are at a security-boundary, you must assume malicious use.
Check everything.

If you are at a module-boundary, you might anticipate programmer error.
Try to catch what you can without compromising your performance and space-use goals, allowing a higher toll if compiled for debugging.

If you are writing module internals, too much checking will just get in the way. On the other hand, too little will make finding and diagnosing bugs unnecessarily hard.

---

Above all else, remember your goals, both functional and non-functional, and exercise due diligence and common sense, as uncommon as it may be.

Answer 3

There are a couple of different ways to go at this.

• The function should clearly document its expectations about the provided data.

• The function should employ defensive programming and validate necessary properties internally.

• It should not be possible to represent invalid states.

However, you're writing C, so things are difficult.

In C, documenting the expectations is probably the most reasonable approach. For example, the init_str() might have the expectation that s points to valid but uninitialized memory (i.e. it can write but not read the values). Or it might have the expectation that s points to zeroed-out memory. Or it might allow s to point to an existing string, which would be freed and then re-initialized.

Depending on the expectations, there is no reasonable validation you can do. How could you tell whether a string is valid or not, except maybe by checking that the pointer is non-null?

Using the type system to make invalid states unrepresentable is something that's very nice in C++. In C, it is a lot trickier since there is no concept of “private” data. However, it is possible to get some degree of encapsulation via incomplete types. Consider this string interface in a header file:

typedef struct string string;
string* str_new(size_t len);
void str_free(string* s);

It is not possible to create a string except through the str_new() function, as string is an incomplete type.

The implementation might then be:

struct string {
  size_t len;
  char data[];  // flexible array member
};

string* str_new(size_t len) {
  if (len > SIZE_MAX - sizeof(string)) return NULL:
  string* s = malloc(sizeof(string) + len);
  if (!s) return NULL;
  s->len = len;
  return s;
}

void str_free(string* s) {
  free(s);
}

It is however possible to obtain a string in an invalid state, such as with a use-after-free.

In some cases tricks with incomplete types are worth it, in particular if all-zero is not a good default value for a type. But for a string, probably not. I would either document the function's expectations clearly, or make the business case to switch to C++ which has a much stronger type system that allows for much better safety.

Answer 4

In my experience, its best to rephrase the idea of "assume s->str is a valid malloc'd string" into one of the phrasings which captures not just the assumption, but what happens if it fails.

The weakest notation I'd recommend is using the term "preconditions." Preconditions are things that must be met in order for the algorithm to work. Saying "Preconditions: s->str is a valid malloc'd string" is the formal way of saying that you're assuming it to be true. Specifically it states that you're really not considering what happens if s->str is not a valid string. If you use "assume," it may be ambiguous (with the different meanings below), but preconditions is a formal phrasing with a well understood meaning.

If we want to go further, the next step is to try to explain what happens if the pre-conditions are not met when the function is called. Fortunately, there are some accepted wordings that can be used for this. The first on the list is "undefined behavior." This is the nastiest of these formal wordings. If I state that "if s->str is not a valid string allocated with malloc() the behavior of the function is undefined," that has a clear meaning of "absolutely anything can happen." In your case, this is likely the behavior you will see. If you access memory that isn't allocated, C++ calls that undefined behavior. Undefined behavior is really bad news. It can cause any effect, including affecting completely unrelated code (such as writing data over the top of other functions' data). If it crashes when you do, that means you got lucky. If it gives you wrong answers several seconds later in an unrelated function, it may take a long time to figure out what happened.

A kinder guarantee you can offer is "unspecified" behaviors. Unspecified behaviors are those which aren't reliable, but will always at least be limited to a known set. A function that returns an "unspecified integer if s->len is greater than 1 million" can return any integer in that case, but the rest of the behaviors are understood. Unspecified behavior won't create unexpected surprises in other sections of code... they'll just give an unspecified result.

The most well known use of unspecified behavior in C++ I am aware of is the order of evaluation of arguments. In f(g(), h()), it is unspecified what order g and h are executed in. But you know for certain that either g is executed and then h, or h is executed and then g, and you can plan accordingly. In my own programming experience, I've often used unspecified results to describe what happens when operating on something with 0 elements. Lots of algorithms are well defined for a non-zero number of elements, like a f(1) or f(15), but there just isn't a really good answer for what should happen for f(0) because the math just sort of breaks down. In these cases, I will often elect to say "the result of f(0) is unspecified," and let it be whatever behavior my implementation happens to have (as long as it doesn't invoke any undefined behavior).

One step nicer than that is "implementation defined behavior." This is a less common phrasing. Undefined behavior and unspecified behavior are common phrasings. Implementation defined behavior is less common. As the C++ spec uses it, implementation defined behavior is behavior which is unspecified by the spec, but any given implementation is expected to provide a definition that you can look up. If my above f(0) example was "implementation defined" rather than "unspecified," the result would be the same except that I would be obliged to provide a document that states what value I chose for f(0). In some cases that is easy. In other cases that's hard.

The final level of definition is to actually specify what happens when a particular assumption is violated. In your example case, this may be hard to do, because there's not many things your algorithm can do if s->str is not a valid string which don't become undefined. However, in many cases you can pick a behavior, and then constrain yourself to it.

So what should you do? The answer depends on how the program will be used. But its worth recognizing that these classes of behavior are generally transitive. If I call a function with undefined behavior, the result of my function is undefined, and so on and so forth all the way up to the program -- we say the program has undefined behavior. Likewise, calling a function with an unspecified value typically results in my result being unspecified as well.

So what behavior is acceptable for a program? If this program is just trying to get a grade in a computer science course, even undefined behavior might be acceptable. But if you're writing an industrial application, undefined behavior can be very bad. Undefined behavior in a robot controller can result in someone being killed. So in an industrial setting, those behaviors aren't accepted. This means that anyone using your function must take the time to prove that your function will not cause undefined behavior when called with their arguments.

This can be expensive. As a result, one may wish to redefine the assumptions. I have written code where init_str not only initializes the string, but creates a note in my own data structures that that particular block of memory was initialized with init_str. do_something then checks to make sure that data was actually allocated before accessing the data in the string. If the check fails, it does something specified or unspecified (such as just returning 0)

In some industries, unspecified behavior is not acceptable. In air traffic control, many applications must be deterministic. All behaviors must be specified.

In all situations, the key to answering your question "should I check" is three fold:

• Can I check for the condition? If I can't, then obviously I can't do anything about it.
• Is it affordable to check for the condition? Sometimes I can check, but it takes a ton of CPU time to do it.
• How does it affect the users of my function?

The last one is really key. If you give me a function with undefined behaviors, I have to grapple with that. I may have to be able to develop a proof that my particular use of the function is always defined. That can be so difficult that I may adjust my algorithm just to make the proof easier.

Answer 5

In general, think of the software built in layers. As data passed between layers, some kind of validation should be performed. Once the data passed validation of a layer, then the same validation shouldn't be performed again by the same layer.

So, if both init_str() and you have do_something1(), do_something2(), etc. and they all live in the same layer, then I wouldn't validate the arguments.

But if they are part of library and they will be called from different layer, i.e. user will call init_str() and then call do_something(), then there's a chance that the data was manipulated by the user, so you should validate.

If your concern is specifically on memory allocation, maybe try library like electric fence which might help. Another way is to have init_str() to also return something like a hash in the string structure, to help detect if the structured had been modified illegally.

Answer 6

Minor point: You wrote

s->str=malloc(len*sizeof(char))

As of https://man.openbsd.org/malloc.3

Consider calloc() or the extensions reallocarray() and recallocarray() when there is multiplication in the size argument of malloc() or realloc().

So this could be written

s->str=calloc(len, sizeof s->str[0])

And you might want to check the return value of malloc as well.

Answer 7

I agree with the other answer - if you know the software is checking, then you may not want to always check for valid input. However, since you can never expect the software front-end to continue to keep those controls, I would recommend putting reasonable checks in the function to verify input. And what do I mean by reasonable? Basically if it starts to degrade performance unacceptably, then that is not reasonable - but, if it is only programming time that is the issue, then take the time now - you will thank your self later.

You never know what a user will do when they end up interacting with the interface. For example, I had an interface that allowed only numbers to be entered - but the interface would allow the 'e' symbol to be input from the control - and then the server would freak out because it didn't know what that was as it was expecting an integer. Additionally, this can happen with 'i' if your software allows certain numbers to be input.

On a final note - instead of always checking for all the fields, make a couple reference functions you can use in lots of places, like 'ConfirmInteger' or something like that, then send the field into it and perform all the checks you need - then just reuse the modular function anywhere you need to instead of having to retype, and of course update, that function in multiple places.

Answer 8

Should I assume data passed to my function is accurate?

It depends... If you are writing some C code for a critical device (e.g. neurosurgical robot; then you have to follow standards and use tools like Frama-C) it is different than if you are coding a game (e.g. using libSDL

A possibility is to use so called magic numbers.