This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] [BZ 17273] fix incorrect mount table entry parsing in __getmntent_r()
- From: Roland McGrath <roland at hack dot frob dot com>
- To: "Vladimir A. Nazarenko" <naszar at ya dot ru>
- Cc: "GNU C. Library" <libc-alpha at sourceware dot org>
- Date: Wed, 1 Oct 2014 14:09:41 -0700 (PDT)
- Subject: Re: [PATCH] [BZ 17273] fix incorrect mount table entry parsing in __getmntent_r()
- Authentication-results: sourceware.org; auth=none
- References: <1409319124-31716-1-git-send-email-naszar at ya dot ru> <20140829215816 dot D05202C3A30 at topped-with-meat dot com> <54014DDC dot 7090005 at ya dot ru>
> 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.