Re: [PATCH 1/9] Add has_extension()
- From
Fredrik Kuivinen <freku045@student.liu.se>
- Date
- Aug 10, 2006, 21:47 UTC
- Message-ID
- <20060810214718.GD13446@c165.ib.student.liu.se>
- In-Reply-To
- <44DB7E01.4000901@lsrfire.ath.cx>
On Thu, Aug 10, 2006 at 08:42:09PM +0200, Rene Scharfe wrote:
Show 25 quoted lines
> Fredrik Kuivinen schrieb:
> > On Thu, Aug 10, 2006 at 05:02:30PM +0200, Rene Scharfe wrote:
> >> +static inline int has_extension(const char *filename, int len,
> >> const char *ext) +{ + int extlen = strlen(ext); + return len >
> >> extlen && !memcmp(filename + len - extlen, ext, extlen); +} +
> >
> > Wouldn't this function be much easier to use if len is computed from
> > filename with strlen? (after a quick look through the other patches I
> > couldn't find a call site where filename wasn't NUL-terminated)
>
> Yes, it would be a bit easier, and my first version had only two
> arguments. Then I found out that the length of the first string is
> already known at _all_ potential callsites, using this command to
> identify candidates:
>
> $ grep 'cmp.*"\..*"' *.[ch]
>
> We could add something like this:
>
> #define has_ext(a, b) has_extension(a, strlen(a), b)
>
> to make it easier to use for code that doesn't already determine the
> string length. I think we should add it only after a user has been
> identified, though.
> IMHO the small speed-up isn't worth it. Drop the extra argument and avoid a possible future bug.
- Fredrik