Messages in this thread | | | From | "Starke, Daniel" <> | Subject | RE: [PATCH 1/3] tty: n_gsm: add keep alive support | Date | Wed, 1 Feb 2023 09:17:31 +0000 |
| |
> > 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. 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.
Best regards, Daniel Starke
| |