Hi, I don't know this codebase, so can't comment on the patch, but the same bug in util-linux was solved by ditching scanf.
https://github.com/karelzak/util-linux/commit/e902609400a861dbdb47d5c3eb98b951530bf01d https://github.com/karelzak/util-linux/commit/e3782bf6776dcef329b09f4324e1be680f690f3c Zbyszek On Tue, Apr 09, 2019 at 07:15:04PM -0700, Paul Eggert wrote: > Bernhard Voelker wrote: > > +/* Find the next white space in STR, terminate the string there in place, > + and return that position. Otherwise return NULL. */ > + > +static char * > +terminate_at_blank (char const *str) > +{ > + char *s = NULL; > + if ((s = strchr (str, ' ')) != NULL) > + *s = '\0'; > + return s; > +} > > Since the function modifies its argument, the argument type should > be char *, not char const *. Also, the code has an assignment in an > 'if' conditional and the comment is not quite right. Better is: > > /* Find the next space in STR, terminate the string there in place, > and return that position. Otherwise return NULL. */ > > static char * > terminate_at_blank (char *str) > { > char *s = strchr (str, ' '); > if (s) > *s = '\0'; > return s; > } > > >+ if (! (blank = terminate_at_blank (mntroot))) > >+ continue; > > Avoid assignments in 'if' conditionals. Better is: > > blank = terminate_at_blank (target); > if (! blank) > continue; > > + if (*source == ' ') > + { > + /* The source is an empty string, which is e.g. allowed for > + tmpfs: "mount -t tmpfs '' /mnt". */ > + *source = '\0'; > + } > + else > + { > + if (! (blank = terminate_at_blank (source))) > + continue; > + } > > Since 'blank' is not used later, surely these 11 lines of code can > be simplified to 2 lines: > > if (! terminate_at_blank (source)) > continue; > > >+ int mntroot_s; > >+ char *mntroot, *blank, *target, *dash, *fstype, *source; > > I suggest using C99-style declaration-after-statement style rather > than this old-fashioned C89-style declarations-at-start-of-block > style, just for the changed part of the code anyway. >