Re: [PATCH 1/9] Add has_extension()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 10, 2006 at 08:42:09PM +0200, Rene Scharfe wrote:
> 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

-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]