Is while(*p++) dangerous?

Question

Is using while(*p++) for checking if an array has more elements dangerous?

Could there be problems if on the next memory location there is some value and this value is not a part of an array.

This simple code:

#include <stdio.h>

void f(float *p) {
    printf("%p : %.f\n", p, *p);
    while(*p++){
        printf("%p : %.f\n", p, *p);
    }
    printf("%p : %.f\n", p, *p);
}

int main() {
    float p[] = {2.2, 3.2};
    f(p);
    return 0;
}

gives this output:

0x7fffed4e8f10 : 2
0x7fffed4e8f14 : 3
0x7fffed4e8f18 : 0
0x7fffed4e8f1c : 0

So, if on 0x7fffed4e8f18 the value was ≠ 0 would it make my code wrong?

Answer 1

As soon as p goes outside of the bounds the array, dereferencing it invokes undefined behavior. So yes, doing this is dangerous.

Answer 2

There are already a lot of answers that explain the undefined behavior that arises from reading past the array's boundaries pretty well. But there is yet another (potential) problem nobody has mentioned yet.

If your array can contain 0 as a valid value, you'll assume the wrong size. That is,

void
f(float * p)
{
  do
    {
      printf("% .01f\n", *p);
    }
  while(*p++);
}

int
main()
{
  float p[] = {-1.0f, -0.5f, 0.0f, 0.5f, 1.0f};
  f(p);
  return 0;
}

will output

-1.00
-0.50
 0.00

and “overlook” the remaining elements. While this particular example does not invoke undefined behavior, it might not be what you've expected.

I guess you've come over this pattern with string programming. By convention, we require a character array that is meant to represent a text string to

1. not contain any NUL bytes and
2. be terminated by placing a NUL byte after the last byte.

If both requirements are fulfilled, doing

void
roll_all_over_it(char * string)
{
  while(*string);
    {
      putchar(*string++);
    }
}

is safe by contract.

This makes using strings a little more convenient since we don't have to maintain an integer alongside every string to keep track of its length. On the other hand, it has made quite a few (carelessly written) programs vulnerable to buffer overrun attacks and there are other problems that arise from this assumption. See for example the discussion of fgets in the GNU libc manual:

Warning: If the input data has a null character, you can't tell. So don't use fgets unless you know the data cannot contain a null. Don't use it to read files edited by the user because, if the user inserts a null character, you should either handle it properly or print a clear error message. We recommend using getline instead of fgets.

Answer 3

Yes, it is: your function takes a pointer argument, but you're not checking to make sure it's not a NULL-pointer. Dereferencing a null pointer is not allowed.
As others have pointed out: dereferencing an invalid pointer (out of bounds) results in undefined bahviour, which is bad.

Also, you are using the correct format string to print a pointer (%p), but compile your code with -Wall -pedantic. printing a pointer value is one of the few cases where you have to cast a pointer to void *. IE change:

printf("%p : %.f\n", p, *p);

to

printf("%p : %.f\n", (void *) p, *p);

update:
In response to your comment: it would seem that you're actually trying to determine the length of an array that is passed as an argument. The simple fact of the matter is that you can't. An array decays into a pointer. A pointer is not an array, and therefore, so you can't determine the length of the original array. At least: not reliably.
If you are working on an array, and you want to know its length: have the caller of your function pass the length as an argument:

void f(float *arr, size_t arr_len)
{
    if (arr == NULL)
        exit( EXIT_FAILURE );//handle error
    //do stuff
}

In short, your function relies heavily on there being a 0 after the array. The function itself can be invoked passing NULL, too and dereferencing null is illegal. So yes, your code is dangerous.

//example setup
float foo = 123.4f
float *bar = malloc(123 * sizeof *foo);//<-- uninitialized memory, contains junk
//omitting if (bar == NULL) check, so bar might be null
//dangerous calls:
f(&foo);
f(bar);//<-- bar could be null if malloc failed, and contains junk if it didn't dangerous
f(NULL);

Answer 4

In C language there's only one way to determine how many elements an array has: it is to look up that value from in original array type, the one that was used in array definition. Any other attempts to figure out the number of elements by some invented run-time mechanisms are useless.

In your specific example, function f has no access to the original array type (since you converted your array to pointer as you were passing it to f). This means that it is impossible to restore the original size of the array inside f, regardless of what you do.

If you insist on passing your array to f as a float * pointer, a good idea would be to pass the original array size manually from the caller (i.e. from main), as an extra parameter of function f.

The "terminating value" technique, where you use some sort of special element value to designate the last element of the array (zero, for example) can also be used for that purpose, but is generally worse that just passing the size from the outside. In any case, you have make sure to include that terminating value into the array manually. It won't appear there by itself.

Answer 5

Yes, the way you have written your code yields undefined behavior if there is no zero element in your array.

That is, while(*p++) in and of itself is not bad, but applying it to an array that you have not explicitly terminated with a zero element is. I. e. the following version is safe:

#include <stdio.h>

//p must point to a zero terminated array
void f(float *p) {
    printf("%p : %.f\n", p, *p);
    while(*p++){
        printf("%p : %.f\n", p, *p);
    }
    //printf("%p : %.f\n", p, *p);    //p points past the end of the array here, so dereferencing it is undefined behavior
}

int main() {
    float array[] = {2.2, 3.2, 0};    //add a terminating element
    f(array);
    return 0;
}

However, I usually prefer explicitly passing the size of the array:

#include <stdio.h>

void f(int floatCount, float *p) {
    printf("%p : %.f\n", p, *p);
    for(int i = 0; i < floatCount; i++) {
        printf("%p : %.f\n", p+i, p[i]);
    }
}

int main() {
    float array[] = {2.2, 3.2};
    int count = sizeof(array)/sizeof(*array);
    f(count, array);
}

Btw: Arrays are not pointers, but they decay into pointers when you pass them to a function. I changed the naming to reflect this.

Answer 6

What you did is indeed highly dangerous !

You make no control of the last element of the array to be null, and you dereference a pointer that will be after the last element of your array. Two points to Undefinit Behaviour !

Here is what you should do :

#include <stdio.h>

void f(float *p) {
    printf("%p : %.f\n", p, *p);
    while(*p++){
        printf("%p : %.f\n", p, *p);
    }
    /* because of ++, p is now one element to far : stop and do not print ... */
}

int main() {
    float p[] = {2.2, 3.2, 0}; /* force a 0 as a last element marker */
    f(p);
    return 0;
}