3

I am writing a Unit Test that is similar to this code, and I am trying to test my values as I set them, so that I know what is going on. I don't understand why the ptr values are not being set to 1, when I run the following code. Instead when I run this it gives me an output of 10, 64, 0, 0.

Any explanation or advise would be greatly appreciated!

#include <stdio.h>
#include <stdbool.h>

typedef struct
{
    bool bOne;
    bool bTwo;
    bool bThree;
    bool bFour;
} items;

int main()
{
    items item;
    item.bOne = 0;
    bool *ptr = &(item.bOne);

    for(int i = 0; i < sizeof(items)/sizeof(bool); i++)
    {
        *ptr = 1;
        *ptr++;
        printf("ptr value = %d\n", *ptr);
    }
    return 0;
}
dsb4591
  • 39
  • 1
  • 4
  • 1
    `*ptr++;` invokes undefined behaviour, because it does not point to an array. It also increments `ptr`, which points to uninitialized memory afterwards. So the `printf` output is garbage. – mch Nov 20 '17 at 16:35
  • 1
    For one thing your printf statement is incorrect because you use `%d`, which expects an `int`, but you pass it a `bool`. Try this: `printf("ptr value = %d\n", (int)*ptr);` – Adrian Nov 20 '17 at 16:35
  • 1
    `*ptr++;`....?? "warning: value computed is not used [-Wunused-value]". – Sourav Ghosh Nov 20 '17 at 16:35
  • You're not printing out the member you just initialized. – John Bode Nov 20 '17 at 16:35
  • for help: http://en.cppreference.com/w/c/language/operator_precedence – Sourav Ghosh Nov 20 '17 at 16:36
  • 1
    1) Turn on warnings. 2) Read the warnings. – klutt Nov 20 '17 at 16:38
  • You cannot iterate through a `struct` like this. It's UB. – Jabberwocky Nov 20 '17 at 16:40
  • Exactly you cant simply use `sizeof` and gamble on the structure member's poosition -the padding the order of declaring - all matters – user2736738 Nov 20 '17 at 16:43
  • Why on earth do you wanna do this? – klutt Nov 20 '17 at 16:48
  • Why increment `ptr` and then print `*ptr` that has not been set? – chux - Reinstate Monica Nov 20 '17 at 16:49
  • @Adrian `printf("ptr value = %d\n", *ptr);` is fine. `bool` get promoted to `int` as part of a `...` argument. The `(int)` in `printf("ptr value = %d\n", (int)*ptr);` is [WET](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself), yet casting has no functional harm either. – chux - Reinstate Monica Nov 20 '17 at 16:57
  • @Adrian: types shorter than an `int` will automatically be passed to `printf()` as an `int`, and `float` values are automatically passed as `double`, because it appears in the variadic part of the argument list. – Jonathan Leffler Nov 20 '17 at 17:04
  • A different approach would be to write a function `test_bool(bool*)` and then call it 4 times. (Not that I immediately see the need to test that a `bool` can be set to 0 and 1). – Bo Persson Nov 20 '17 at 17:17
  • Possible duplicate of [Iterating over same type struct members in C](https://stackoverflow.com/questions/1869776/iterating-over-same-type-struct-members-in-c) – phuclv Aug 21 '18 at 10:20

2 Answers2

3

In *ptr++, the ++ has higher precedence than the *, so this post-increments the pointer, and reads and discards the value originally pointed to. Now that the pointer has been incremented, you are reading uninitialized memory in the printf. If your intention was to increment the value being pointed at, try:

(*ptr)++;

Or

ptr[0]++;

Edit

Hmm, since your loop bound is based upon the number of bools that fit in the size of the struct, maybe your intention was to increment the pointer. In which case, you don't need to dereference it at the same time, and you shouldn't expect to get anything meaningful in the printf. Also, as pointed out, since the struct is not an array, you will be wandering into undefined behavior land since the compiler may decide to add padding:

From C99 §6.7.2.1:

12 Each non-bit-field member of a structure or union object is aligned in an implementation- defined manner appropriate to its type.

13 Within a structure object, the non-bit-field members and the units in which bit-fields reside have addresses that increase in the order in which they are declared. A pointer to a structure object, suitably converted, points to its initial member (or if that member is a bit-field, then to the unit in which it resides), and vice versa. There may be unnamed padding within a structure object, but not at its beginning.

pat
  • 12,587
  • 1
  • 23
  • 52
  • Well I am working on embedded system, and unit testing code that's already been written. I've never worked with embedded code so this is quite complicated to test and write. The typedef struct is already made of bool values. The function I am testing deals with 28 if statements that each have a struct value evaluated, and if true a word variable is then set to a hex value, and the bit-wise operation is performed on it, based on if it's true or false. What I am trying to do as a result is perform a walking 1's and 0's operation. – dsb4591 Nov 20 '17 at 16:58
  • @dsb4591 if you are sure your compiler doesn't add any padding, then you can do this. Otherway see if your compiler has some #pragmas that allow you to specify the packing of structs. – Jabberwocky Nov 20 '17 at 17:00
  • @MichaelWalz I will definitely look into to your suggestion. Thank you! – dsb4591 Nov 20 '17 at 17:02
2

You have two major problems.

First, you have a logical error in that you advance your pointer before printing out the value it points to; instead of printing the value of the thing you just set, you're printing the value of the next uninitialized member. You need to reverse the order of the advance and print statements like so:

printf("ptr value = %d\n", *ptr);
ptr++; // note no * operator

Or just combine the operation in one expression:

printf("ptr value = %d\n", *ptr++);

This assumes that you can use %d to print a bool value - I'm not sure about that offhand (may have to explicitly cast the argument to (int)).

But you have a bigger problem - it's not guaranteed that you can iterate through the members of a struct type with a pointer like this. Even though all the members are the same type, it's not guaranteed that there won't be padding between members (it depends on the size of the bool type and the alignment requirements of your platform).

Your code may work as intended, or it may lead to corrupted data within your struct instance, or any number of other issues. The safer thing to do is use a separate array of pointers to those members, since it is guaranteed that you can iterate through an array with a pointer:

bool *members[] = { &item.bOne, &item.bTwo, &item.bThree, &item.bFour, NULL };

bool *ptr = members[0];
while ( ptr )
{
  *ptr++ = 1;
}

If you still want output, you'd write

*ptr = 1;
printf( "ptr value = %d\n", *ptr++ );

You could get creative and combine the assignment and pointer advance within the print statement:

printf( "ptr value = %d\n", (*ptr++ = 1) );

but that would probably get you hit.

John Bode
  • 119,563
  • 19
  • 122
  • 198