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.
Ken
From f679462937faf263de682c47f8d8e73b0c7e4136 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
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
--- a/winsup/utils/dump_setup.cc
+++ b/winsup/utils/dump_setup.cc
@@ -48,21 +48,6 @@ find_tar_ext (const char *path)
return 0;
}
-static char *
-base (const char *s)
-{
- if (!s)
- return 0;
- const char *rv = s;
- while (*s)
- {
- if ((*s == '/' || *s == ':' || *s == '\\') && s[1])
- rv = s + 1;
- s++;
- }
- return (char *) rv;
-}
-
/* Parse a filename into package, version, and extension components. */
int
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[0] = f.what[0] = '\0';
- p = base (fn);
+ p = fn;
for (ver = p; *ver; ver++)
if (*ver != '-')
continue;
--
2.14.2
