[PATCH] cygcheck: Fix parsing of file names containing colons
Wed Oct 25 14:56:00 GMT 2017
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)
>>>>> + if ((*s == '/' || *s == '\\') && s)
>>>> 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
>>> 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.
-------------- next part --------------
From f679462937faf263de682c47f8d8e73b0c7e4136 Mon Sep 17 00:00:00 2001
From: Ken Brown <firstname.lastname@example.org>
Date: Tue, 24 Oct 2017 18:21:53 -0400
Subject: [PATCH] winsup/utils/dump_setup.cc: Remove the function 'base'
This was called only on filenames in /etc/setup/installed.db, which
are all basenames anyway. Moreover, base wasn't correctly handling
filenames containing colons.
winsup/utils/dump_setup.cc | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/winsup/utils/dump_setup.cc b/winsup/utils/dump_setup.cc
index 320d69fab..4415954f9 100644
@@ -48,21 +48,6 @@ find_tar_ext (const char *path)
-static char *
-base (const char *s)
- if (!s)
- return 0;
- const char *rv = s;
- while (*s)
- if ((*s == '/' || *s == ':' || *s == '\\') && s)
- rv = s + 1;
- return (char *) rv;
/* Parse a filename into package, version, and extension components. */
parse_filename (const char *in_fn, fileparse& f)
@@ -79,7 +64,7 @@ parse_filename (const char *in_fn, fileparse& f)
strcpy (f.tail, fn + n);
fn[n] = '\0';
f.pkg = f.what = '\0';
- p = base (fn);
+ p = fn;
for (ver = p; *ver; ver++)
if (*ver != '-')
More information about the Cygwin-patches