> > > > index cb8693b39cb7..b64360aca1f9 100644 > > > > --- a/include/uapi/linux/gsmmux.h > > > > +++ b/include/uapi/linux/gsmmux.h > > > > @@ -19,7 +19,8 @@ struct gsm_config > > > > unsigned int mtu; > > > > unsigned int k; > > > > unsigned int i; > > > > - unsigned int unused[8]; /* Padding for expansion without > > > > + unsigned int keep_alive; > > > > + unsigned int unused[7]; /* Padding for expansion without > > > > > > "unsigned int" is not really a valid uapi variable type. > > > > > > Shouldn't this be __u32 instead? > > > > I know but changing it to a fixed size data type may break compatibility > > as this may change the overall size of the structure. > > Will it? It shouldn't that's why using the correct data types is > essencial. Well, unsigned int is defined to be at least 16 bit. Using __u32 will break systems where this is true. I am not sure if the Linux kernel targets any system which defines unsigned int with 16 bit. But sure, I can change it to __u32. > > This is why I > > took a field out of the "unused" array for the "keep_alive" parameter. > > A value of zero disables keep-alive polling. > > > > > Should you document this field as to what the value is and the units as > > > you are creating a new user/kernel api here. > > > > I will add a comment here. Comments for the other fields remain subject to > > another patch. > > > > > And finally, "unused" here is being properly checked to be all 0, right? > > > If not, then this change can't happen for obvious reasons :( > > > > This was not the case until now. I assumed there was some coding guideline > > that unused fields need to be initialized to zero. Obviously, checking it > > prevents misuse here. I will add relevant checks for this. > > If the value was not checked previously, then you can not use the field > now, otherwise things will break, sorry. Those are useless fields and > should be marked as such :( What is the way forward here? Should I introduce a complete new ioctl? Or should I use a different size for this structure to break existing code intentionally? Does this mean that we cannot extend this structure at all in the future? I had planned another extension here to properly support parameter negotiation. In case we need to keep the structure as it is: Would a comment be sufficient to mark this field accordingly? Best regards, Daniel Starke
