Re: [PATCH] [BZ 17273] fix incorrect mount table entry parsing in __getmntent_r()

[Roland McGrath <roland at hack dot frob dot com>]

> On 30.08.2014 08:58, Roland McGrath wrote:
> > Please write a test case.
> > 
> Can I modify existing test or I must create new one? Is following patch OK?

If it is substantially simpler to modify an existing test, that is the
right thing to do.

> Can I resend fixed patch and test case as *one* patch?

Yes, that is always the right way to do it.  That way when you say that
you've run 'make check' and seen no regressions (on some particular
configuration, which you should cite--saying that and having it be true is
a necessary part of submitting any substantive change), that constitutes
testing for the newly-fixed case too.

> 	[BZ #17273]
> 	* misc/mntent_r.c (__getmntent_r): cut off trailing spaces
> 	and tabs from bufer before parsing fstab entry.

Capitalize sentences.

> 	* misc/tst-mntent.c(main): add test for mount entry with 
> 	trailing spaces and tabs.

Space before that paren.

> +       /*part III: test if entry with trailing whitespaces*/

Comments need proper grammar, capitalization, punctuation, and space usage.
Follow the existing examples.

> +      fputs("/foo\\040dir /bar\\040dir auto bind \t \n", fp);

Space before paren.

> +      mnt = getmntent(fp);
> +      mnt = getmntent(fp);

Space before paren.  Did you really mean to call it twice?

> +      if (mnt->mnt_freq != 0 || mnt->mnt_passno != 0)

Check all the fields like the existing case does.

> +	{
> +	  printf("Error mnt_freq = %d and mnt_opts = %d, but zero expected\n",
> +			  mnt->mnt_freq, mnt->mnt_passno);
> +          result = 1;
> +	}

Space before paren.  The second line of the block should be intended the
same as the first, and use a tab for the leading 8 spaces as the first line
does.