[libc-commits] [PATCH] D82724: [libc] Add memchr implementation.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/string/memchr.cpp:18
+  const unsigned char *str = reinterpret_cast<const unsigned char *>(src);
+  const unsigned char ch = static_cast<unsigned char>(c);
+  for (; n && *str != ch; --n, ++str)
----------------
cgyurgyik wrote:
> sivachandra wrote:
> > Do you need the explicit static cast? Without it, wouldn't the compile time implicit cast do the same thing?
> Semantically they're the same thing. I just assumed that when narrowing or widening, its always better to be explicit. 
You still need a cast. I was asking about the explicit static cast, but at least the implicit cast is requried. That is:

```
unsigned char ch = c;
for (; n && *str != ch; --n, ++str)
...
```

Without this, and if `char` type were `signed char`, then this will happen:

```
char c = -1;
char mem[5] = {c, c, c, c, c};
memchr(mem, c, 5);
```

The `unsigned char` values in `mem` are all 255. But, they will get promoted to an `int` value of 255 and then compared with an `int` value of -1. So `memchr` will return `nullptr`.

Sorry for not catching this earlier.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82724/new/

https://reviews.llvm.org/D82724