spi/pwm: fix "uninitialized" warnings

[Livius90]

Fix "uninitialized" warnings which can produced in case of use GCC v12.3.0 with -Wall -Wextra arguments.

[vsergeev]

@Livius90 thanks for the PR. My only concerns are that we shouldn't add *_INVALID states to the end of the enums. What if we legitimately needed to add a new enum member? Generally speaking, we shouldn't change an external API to fix internal warnings.

[Livius90]

@Livius90 thanks for the PR. My only concerns are that we shouldn't add *_INVALID states to the end of the enums. What if we legitimately needed to add a new enum member? Generally speaking, we shouldn't change an external API to fix internal warnings.

It is practical to provide an initialize value for an enum type variable.

https://github.com/vsergeev/c-periphery/pull/51/files#diff-324566633fa8f84c13e8a0e4d2eae73cdf43a89a9db1d75a14cc0bd7b672827bR416

pwm_polarity_t polarity = PWM_POLARITY_INVALID;

What do you prefer to use for initialize an enum variable? If just zero would use, it means it will be initialized for the first enum value. In a more complex code it can cause any issue.

pwm_polarity_t polarity = 0;    /* PWM_POLARITY_NORMAL */

[vsergeev]

It's not practical to put the invalid state after a bunch of valid ones. That's not only confusing, but also breaks the enum for any future additions. I agree with your latter suggestion -- that we might as well just explicitly initialize to 0 (whichever state it corresponds to).

[Livius90]

Do you mean the *_INVALID type should be the first like this?

typedef enum pwm_polarity {
    PWM_POLARITY_INVALID,
    PWM_POLARITY_NORMAL,    /* Normal polarity */
    PWM_POLARITY_INVERSED,  /* Inversed polarity */
} pwm_polarity_t;

What do you prefer now, i can change to use *_INVALID enum types from first position in enum list, or I can just initializing bit_order and polarity to zero without change enums in headers?

pwm_polarity_t polarity = 0;    /* PWM_POLARITY_NORMAL */
...
spi_bit_order_t bit_order = 0;    /* MSB_FIRST */

[vsergeev]

Do you mean the *_INVALID type should be the first like this?

typedef enum pwm_polarity {
    PWM_POLARITY_INVALID,
    PWM_POLARITY_NORMAL,    /* Normal polarity */
    PWM_POLARITY_INVERSED,  /* Inversed polarity */
} pwm_polarity_t;

Yup, exactly. This would have been ideal, but at this point I don't think it's a good idea to break the API.

What do you prefer now, i can change to use *_INVALID enum types from first position in enum list, or I can just initializing bit_order and polarity to zero without change enums in headers?

pwm_polarity_t polarity = 0;    /* PWM_POLARITY_NORMAL */
...
spi_bit_order_t bit_order = 0;    /* MSB_FIRST */

Yeah, this is a good compromise for now. I don't think the comment is actually needed, and it might even be a bit confusing, because the intention is just to give the variable an initialized value rather than set it to the state described in the comment.

[Livius90]

@vsergeev
Done

[Livius90]

@vsergeev
When is it going to be merged?

[vsergeev]

Hmm... I'm not seeing these warnings under GCC 14.2.1 or Clang 18.1.8. Are you still getting the warning? I'm also a little concerned because there are many more uninitialized stack variables in other modules than the ones covered in these commits.

[Livius90]

Hmm... I'm not seeing these warnings under GCC 14.2.1 or Clang 18.1.8. Are you still getting the warning? I'm also a little concerned because there are many more uninitialized stack variables in other modules than the ones covered in these commits.

In my cross-compiler buildflow i use GCC 13.3.0 and cmake is changed to use latest C and C++ standard (much newer then gnu99) and i still have the wranings if i not fix them via a patch.

You can not get this warning in any other uninitialized stack variables because in the other code parts they got any a = ... operation which will solve it, but if the variable is just passed into a pointer arg of a function this warning is happened sure.

[vsergeev]

I'm not ready to advance the C standard beyond gnu99 -- for now it seems like a decent compromise for users on both new and old platforms. Is there any reason you can't compile the library with gnu99 and the rest of your code with a newer standard? The c-periphery ABI in the object files should be compatible.

As for the uninitialized variable changes, I'm not opposed to those in principle.

[Livius90]

I'm not ready to advance the C standard beyond gnu99 -- for now it seems like a decent compromise for users on both new and old platforms. Is there any reason you can't compile the library with gnu99 and the rest of your code with a newer standard? The c-periphery ABI in the object files should be compatible.

As for the uninitialized variable changes, I'm not opposed to those in principle.

You can see more and important warnings if you increasing the C standard. Because it is now 2025, i thought it should be the right time to get much better than gnu99.