This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] cygcheck: Fix parsing of file names containing colons


On Oct 25 10:56, Ken Brown wrote:
> On 10/25/2017 10:19 AM, Corinna Vinschen wrote:
> > On Oct 25 09:38, Ken Brown wrote:
> > > On 10/25/2017 8:19 AM, Corinna Vinschen wrote:
> > > > On Oct 25 14:11, Corinna Vinschen wrote:
> > > > > Hi Ken,
> > > > > 
> > > > > On Oct 25 07:23, Ken Brown wrote:
> > > > > > Up to now the function winsup/utils/dump_setup.cc:base skips past
> > > > > > colons when parsing file names.  As a result, a line like
> > > > > > 
> > > > > >     foo foo-1:2.3-4.tar.bz2 1
> > > > > > 
> > > > > > in /etc/setup/installed.db would cause 'cygcheck -cd foo' to report 4
> > > > > > as the installed version of foo insted of 1:2.3-4.  This is not an
> > > > > > issue now, but it will become an issue when version numbers are
> > > > > > allowed to contain epochs.
> > > > > > ---
> > > > > >    winsup/utils/dump_setup.cc | 2 +-
> > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
> > > > > > index 320d69fab..3922b18f8 100644
> > > > > > --- a/winsup/utils/dump_setup.cc
> > > > > > +++ b/winsup/utils/dump_setup.cc
> > > > > > @@ -56,7 +56,7 @@ base (const char *s)
> > > > > >      const char *rv = s;
> > > > > >      while (*s)
> > > > > >        {
> > > > > > -      if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
> > > > > > +      if ((*s == '/' || *s == '\\') && s[1])
> > > > > 
> > > > > I think this is a simplified way to test for the colon in paths like
> > > > > C:/foo/bar.  Nothing else makes sense in this context.
> > > > > 
> > > > > I'm not sure how much we care, but maybe we shoulkd fix the test to
> > > > > ignore the colon only if it's the second character in the incoming
> > > > > string?
> > > > 
> > > > Not "ignore", but "use as a delimiter" only as 2nd char in the input.
> > > 
> > > I'm not sure the distinction matters in this case, since the function is
> > > just trying to get the base name.  Anyway, how's the attached?
> > 
> > Fine, thanks.
> > 
> > But now that you mention it... why does parse_filename() call base() at
> > all?  The filenames in installed.db are just basenames anyway.  Does
> > that cover an older DB format we don't support anymore, perhaps?
> 
> It looks like parse_filename is more-or-less copied from the function with
> the same name in the setup sources (in filemanip.cc).  In that case there
> might be a good reason to call base; I'll have to look more closely.
> 
> > I just wonder now if we should simply remove base() and the call to it.
> > 
> > Either way, you're right, the colon check is just useless, so your first
> > patch was entirely sufficient.
> > 
> > What do you think?  Stick to your patch or remove base()?
> 
> I vote for removing base.  The attached patch does this.

Pushed.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]