On Tuesday 26 October 2010 16:55, Alexander Shishkin wrote: > When mounting a filesystem without any additional options (data parameter > to the mount(2) syscall), pass NULL instead of an empty string like GNU > mount does. This fixes, for example mounting cgroup fs with bbox mount. > > Signed-off-by: Alexander Shishkin <[email protected]> > --- > util-linux/mount.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/util-linux/mount.c b/util-linux/mount.c > index d1d6889..3e53023 100644 > --- a/util-linux/mount.c > +++ b/util-linux/mount.c > @@ -344,6 +344,7 @@ static long parse_mount_options(char *options, char > **unrecognized) > const char *option_str = mount_option_str; > > if (comma) *comma = '\0'; > + else if (!*options) break; /* no options -- leave */
The comment explains _what_ code does (which is obvious from the code), whereas it needs to explain _why_ it does that. The future reader will spend some time figuring out that if options == "", you skip (let me see...) search in mount_option_str, and appending of "" to the end of unrecognized opts. Which of these two was the problem? And what was the exact mechanism of the problem? And can you give an example "mount ..." which triggers it? _Thats_ what comment should document, not that !*options means "empty string" and break means "leave the loop". Everybody knows that. And it should use // comments, same as the rest of the file. BTW, what if options == "foo,"? What if options == "foo,,bar"? -- vda _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
