Messages in this thread |  | | From | Rasmus Villemoes <> | Subject | Re: [PATCH v2 1/8] lib: string: Introduce strreplace | Date | Tue, 09 Jun 2015 09:17:41 +0200 |
| |
On Tue, Jun 09 2015, Joe Perches <joe@perches.com> wrote:
> On Tue, 2015-06-09 at 01:26 +0200, Rasmus Villemoes wrote: >> Strings are sometimes sanitized by replacing a certain character >> (often '/') by another (often '!'). > [] >> v2: spello fixed, parameters renamed 'old' and 'new' (just so the >> kernel doc aligns nicely, and because that's what python -c >> 'help(str.replace)' uses). Still EXPORT_SYMBOL, not inline (tried it, >> caused more bloat), still called strreplace. > > OK, thanks. I think the chars should be ints though > just for consistency for strchr variants.
I disagree. That way lies subtle (semi)bugs. Quick quiz: What does memscan return below?
char a[1] = {X}; /* for some suitable X */ char *p = memscan(a, a[0], 1);
Here's the kerneldoc+prototype for memscan:
/** * memscan(void *addr, int c, size_t size) - Find a character in an area of memory. * @addr: The memory area * @c: The byte to search for * @size: The size of the area. * * returns the address of the first occurrence of @c, or 1 byte past * the area if @c is not found */
So obviously p==a, right? Wrong. Or rather, wrong when char is signed and X lies outside the ascii range. Or maybe right, if you're on an architecture with its own memscan that DTRT. And 'the right thing' is obviously to use only the LSB of c, which would have been harder to get wrong if c was just a u8 to begin with.
(The only in-tree callers which do not pass an explicit non-negative constant seem to be in drivers/hid/usbhid/usbkbd.c and net/bluetooth/hidp/core.c, and they both pass something from an unsigned char array).
We're stuck with int in the libc functions, but we can do better for new interfaces.
Rasmus
|  |