[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2a6bae242fc547284e5c311cf7d81aae@smtp.hushmail.com>
Date: Sun, 17 Mar 2013 23:14:53 +0100
From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com
Subject: Re: All *2john programs should use basename of filename when put as “login” field
On 17 Mar, 2013, at 22:54 , Lukas Odzioba <lukas.odzioba@...il.com> wrote:
> 2013/3/17 magnum <john.magnum@...hmail.com>:
>> As far as I know there are two problems:
>> 1. Some versions of basename() modify the original string, some do not. This is only a problem if we want to keep the original full name as well.
>> 2. Some versions of basename() may return NULL on error. This problem might be more or less academic for this use.
>>
>> The current gpg2john code handles both cases. A generic version in misc.c or maybe in path.c is a good idea. Supporting an arbitrary list of extensions to strip would be powerful of course.
>
> misc.s seems to be better place.
> What do you think about this version:
...
> char *get_basename(const char *path, const char *extensions[], int count)
> {
> assert(path);
> assert(*path);
I think we should avoid asserts other than for debugging. We could opt to return a valid null string:
if (!path || !*path)
return "";
> static char retmem[64];
I think I'd use at least 256 here.
> char *pathdup = NULL;
> char *base = NULL;
> int retsize,i,extlen,baselen = 0;
> pathdup = strdup(path);
> if(!pathdup)
> goto error;
> base = basename(pathdup);
> if(!base || !*base)
> goto error;
> baselen = strlen(base);
>
> while(count--) {
> extlen=strlen(extensions[count]);
> if ( baselen > extlen &&
> !strcmp(&base[baselen-extlen],extensions[count]) ) {
> base[baselen-extlen]=0;
> break;
> }
> }
>
> retsize = strlen(base) + 1;
> memcpy(retmem, base, retsize<64?retsize:64);
The above does not guarantee a null termination. Replace the two lines with strnzcpy(retmem, base, sizeof(retmem)), conveniently also located in misc.c.
> free(pathdup);
> return retmem;
>
> error:
> if(pathdup)
> free(pathdup);
> return path;
> }
The rest looks fine to me, but I'm not exactly a skilled code reviewer :)
magnum
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.
