This is the mail archive of the
mailing list for the Cygwin project.
Re: [PATCH]Package extention recognition (revision 1)
----- Original Message -----
From: "Robert Collins" <email@example.com>
To: "Michael A Chase" <firstname.lastname@example.org>
Sent: Friday, January 25, 2002 00:53
Subject: Re: [PATCH]Package extention recognition (revision 1)
> On Fri, 2002-01-25 at 13:58, Michael A Chase wrote:
> > ----- Original Message -----
> > From: "Christopher Faylor" <email@example.com>
> > To: <firstname.lastname@example.org>
> > Sent: Thursday, January 24, 2002 17:51
> > Subject: Re: [PATCH]Package extention recognition
> > > On Thu, Jan 24, 2002 at 05:48:35PM -0800, Michael A Chase wrote:
> > > >I noticed that find_tar_ext() always returns after checking for
> > ".tar.bz2"
> > > >and ".tar.gz" so it never gets to the check for ".tar". As long as I
> > > >fixing that, it seemed like a good time to add ".cwp" as an accepted
> > > >extension.
> > >
> > > Haven't we already debated this issue? I don't see any reason to
> > > a .cwp on the world and I can't imagine why we'd ever want a plain
> > > rather than a .tar.bz2.
> > The last discussion I saw on ".cwp" kind of wandered off when someone
> > offered a patch that was more complicated than necessary. It seemed
> > good idea (along with .deb or rpm) to avoid the next round of 'Why
> > the install I did with WinZip not work?', but I can easily change the
> > remove both.
> .cwp was an idea to avoid Winzip users - but .bz2 does that too, as will
> deb or rpm. The previous patch wasn't rejected due to complexity, but
> due to incompleteness - it was a partial factoring, but didn't make the
> code easier to maintain or extend, as generic magic number support
> (which is present now) does. (ie when you pass a stream to
> archive::open, the extension doesn't matter, only the content. Ditto for
".bz2" only 'protects' WinZip users until WinZip starts handling bzip2
files. That may never happen, but ".cwp" (or ".rpm" or ".deb") isn't
vulnerable to WinZip improvements.
> > The current un-patched code leaves off ".tar" inadvertently.
> If that is the filemanip code, it seems fine to me, or perhaps you got
> your patch arguments the wrong way around for that file?
> Ok, some feedback: strdup is deprecated except where it's needed to
> interface with a C-library that deallocates via the malloc family.. Use
> new char[n+1] and strcpy. Eventually we'll either roll our own string
> class, grab an existing one, or use the STL.
> Regarding the source file tests, that approach looks ok. Long term it
> will be something like:
> find a binary,
> extract metadata,
> check for the source file(s) (which are explicitly named in the
> I'll apply the lot if you'll
> 1) cleanup the strduping,
The strdup() was in one of the lines I didn't change. I'll change it.
While I'm at it, I'll look for other strdup() calls in the file and change
them as well.
> 2) Answer my query about filemanip.
The '-' lines below are from the unpatched filemanip.cc. Notice the ';' at
the end of the 'if' lines. That causes the return after the ".tar.gz" 'if'
statement to always be executed. As a result, the test for ".tar" never
My original patch (which I forgot to attach to the first message) included a
test for ".cwp" to test the waters. Should I add it back? If I do, should
I also add ".rpm", and ".deb"?
--- filemanip.cc-0 Thu Jan 24 16:43:57 2002
+++ filemanip.cc Thu Jan 24 16:22:08 2002
@@ -69,10 +69,12 @@ find_tar_ext (const char *path)
char *ext = strstr (path, ".tar.bz2");
return (end - ext) == 8 ? ext - path : 0;
- if ((ext = strstr (path, ".tar.gz")));
- return (end - ext) == 7 ? ext - path : 0;
- if ((ext = strstr (path, ".tar")));
- return (end - ext) == 4 ? ext - path : 0;
+ if ((ext = strstr (path, ".tar.gz")))
+ return (end - ext) == 7 ? ext - path : 0;
+ return 0;
/* Parse a filename into package, version, and extension components. */
** I normally forward private questions to the appropriate mail list. **
Give a hobbit a fish and he eats fish for a day.
Give a hobbit a ring and he eats fish for an age.