[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130319005311.CTWAU.646755.imail@eastrmwml107>
Date: Mon, 18 Mar 2013 20:53:11 -0400
From: <jfoug@....net>
To: john-dev@...ts.openwall.com
Cc: Lukas Odzioba <lukas.odzioba@...il.com>
Subject: Re: basename portability 'fix'.
Let me check the code, to be 100% sure the correct stuff was in the patch.
As for only using basename_r, the reason I kept both a basename and basename_r, is that by simply changing from basename to jtr_basename is all that is needed. In fact, we could add this to misc.c
#undef basename
#define basename(a) jtr_basename(a)
and then all you need to do is make sure that misc.h is included, then a call to char *base = basename(name); would work as expected (we might have to drop the gcc header that defines basename).
> Merging may be not a good idea after all. Maybe it would be better to
> create: strip_extensions(name,ext,num); and call this like that:
> strip_extensions(jtr_basename(path),ext,num);
I think this a better idea also. the basename I did is a little tricky, since it tries pretty hard to do the right thing, even with different OS requirements (different path separators, optional drive letter, etc).
NOTE, the code as I wrote it, will also give the last path element as the basename, if only a path was given. This is the same as how the gcc basename is defined. So /tmp/usr/ returns usr. This keeps the original intent of basename, and in JtR's usage should not cause any problems.
It really was build to do the same as the manpage I got on Linux, EXCEPT using a const char name param, and HONORING that. The gcc one is horrible. Simply doing something like: char *x = basename("/usr/local/"); could (and likely will) crash the program, without looking like it could/should. If the program tries to modify that constant string, and the OS actually forces strings to be in a const segment, it will crash. Worse than that, if the string DOES get modified, then any other part of the program that used that const string, would get different data sent to it, than expected. In generral any function with a char * param that modifies it, IF used in a library form, where many developers will use it, is a large design failure.
Jim.
---- Lukas Odzioba <lukas.odzioba@...il.com> wrote:
> 2013/3/19 magnum <john.magnum@...hmail.com>:
> >
> > I guess we can just drop all statements that declare or set name?
>
> 1) I think so, and moreover in my opinion it would be better if we
> change param from _name to just name;
> 2) I really am not convinced that we need two versions of this
> function. Since it is not performance critical I would leave just
> thread safe one.
> 3) Great job, thx!
>
> > If this is safer than our portability workarounds, I suppose Lukas could merge it to his get_basename() code that also strips an arbitrary list of extensions.
>
> Merging may be not a good idea after all. Maybe it would be better to
> create: strip_extensions(name,ext,num); and call this like that:
> strip_extensions(jtr_basename(path),ext,num);
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
