Re: strlen

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

 



On 7/8/21 12:07 PM, Jonny Grant wrote:
Thank you for your reply.

We can't guarantee safestrlen() won't be called with NULL. So because strlen() itself doesn't check for NULL in C standard we'd need to call the wrapper so that NULL can be checked for.

I'd like to avoid the compiler removing certain execution paths.
I'd rather keep all code paths, even if they are not taken, just in case a NULL pointer creeps in due to an external device that is connected to an embedded system.


Probably this would work:

size_t __attribute__((optimize("O0"))) safestrlen(const char * s)
{
     if (NULL == s) return 0;
     else return strlen(s);
}

I don't think you don't need that. Unless there's a bug in GCC, it shouldn't optimize that path unless it is 100% sure that it will never be called.

Moreover, I recommend you to optimize as much as possible.
Even though NULL is possible in your code, I guess it's unlikely.

Also, calling a function safe is too generic.
I'd call it with the suffix null, as it act different on null.

Also, I recommend avoiding 'size_t' (and any other unsigned types, BTW).
See <https://google.github.io/styleguide/cppguide.html#Integer_Types>.
Use the POSIX type 'ssize_t'.
That also allows differentiating a length of 0 (i.e., "") from an invalid string (i.e., NULL), by returning -1 for NULL.

Recommended implementation (requires C99 or later due to the usage of inline):

[
// compiler.h

#define likely(x)    __builtin_expect((x), 1)
#define unlikely(x)  __builtin_expect((x), 0)
]


[
// strlennull.h

#include <string.h>
#include <sys/types.h>

#include "compiler.h"

[[gnu::pure]]
inline ssize_t strlennull(const char *s);

inline ssize_t strlennull(const char *s)
{
	if (unlikely(!s))
		return -1;
	return strlen(s);
}
]


[
// strlennull.c

#include "strlennull.h"

#include <sys/types.h>

extern inline ssize_t strlennull(const char *s);
]


BTW, if the input is so untrusted that NULL may be possible, I guess it might as well contain invalid strings (non-NUL-terminated), for which strlen(3) would behave as bad or worse. So I recommend having a look at strnlen(3) and maybe implement strnlennull() instead of strlennull().


Unless you _know_ there's a compiler bug that doesn't allow you to optimize, please optimize. Otherwise, if it's just a bit of paranoia, you could as well not optimize any code at all. Specifically not optimizing this code by the use of pragmas or attributes is *wrong*. Just revise the preprocessor output, the compiler output, and also try introducing a NULL at run time, to check that everything's fine.


I also use 'volatile' for reads/writes to addresses that I don't want to be optimized out.

Are you sure you don't want to optimize? Are those addresses hardware I/O or interrupts?

Maybe you just want membarrier(2).



What I recommend you to do from time to time, to make sure you don't miss any warnings, is compile the whole project first with '-O3' and then with '-O0'.  If you are a bit paranoic, sporadically you can try all of them : '-Og', '-O0', '-O1', '-Os', '-O2', '-O3', '-Ofast' but I don't think that is necessary.  Of course, always use '-fanalyzer' (GCC 10 and above).

Yes, I am looking forward to David Malcom's -fanalyzer when Ubuntu LTS next upgrades, I'm on gcc 9.3 today. But -fanalyzer is only for C anyway.. much of of code base I work with is compiled as C++ so I can't use -fanalyzer yet.

You can install gcc-10 for Bionic: apt-get install gcc-10
I recommend it.  It finds bugs very deep in the code.


Cheers,

Alex


--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/



[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux