Re: [PATCH 1/1] string: Add stpecpy(3)

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3160 bytes --]

Hi Noah,

On 12/25/22 02:52, Noah Goldstein wrote:

>> char *
>> stpecpy (char *dst, char *end, const char *restrict src)
>> {
>>     size_t dsize;
>>     size_t dlen;
>>     size_t slen = strlen (src);
> 
> Imo move `dst == end` and `dst == NULL` check before strlen


That's a valid optimization.  Having strlen(3) before has the advantage that you 
make sure that your strings are strings, as strlcpy(3) does.  But since we're 
inventing stpecpy(3), we can choose to be faster.  If anyone wants to instrument 
their code, they can add a temporary wrapper that does that.

> and change strlen to `strnlen(src, dsize)`.

About strnlen(3), I have doubts.  Isn't strlen(3) faster for the common case of 
no truncation or little truncation?  strnlen(3) would optimize for the case 
where you truncate by a large difference.

>>     bool   trunc = false;
>>
>>     if (dst == end)
> 
> Out of curiosity what if `end < dst`?

The behavior is undefined.  That's by design.  In the definition of stpecpy(3) I 
have currently in libstp, I even tell the compiler to optimize on that condition:
<http://www.alejandro-colomar.es/src/alx/alx/libstp.git/tree/include/stp/stpe/stpecpy.h#n33>


alx@asus5775:~/src/alx/libstp$ grepc -tfd stpecpy
./include/stp/stpe/stpecpy.h:21:
inline char *stp_nullable
stpecpy(char *stp_nullable dst, char *end, const char *restrict src)
{
	bool    trunc;
	size_t  dsize, dlen, slen;

	slen = strlen(src);

	if (dst == end)
		return end;
	if (stp_unlikely(dst == NULL))  // Allow chaining with stpeprintf().
		return NULL;
	stp_impossible(dst > end);

	dsize = end - dst;
	trunc = (slen >= dsize);
	dlen = trunc ? dsize - 1 : slen;
	dst[dlen] = '\0';

	return mempcpy(dst, src, dlen) + trunc;
}
alx@asus5775:~/src/alx/libstp$ grepc -tm stp_impossible
./include/stp/_compiler.h:14:
#define stp_impossible(e)   do                                                \
{                                                                             \
	if (e)                                                                \
		stp_unreachable();                                            \
} while (0)
alx@asus5775:~/src/alx/libstp$ grep -rnC1 define.stp_unreachable
include/stp/_compiler.h-28-#if defined(unreachable)
include/stp/_compiler.h:29:# define stp_unreachable()  unreachable()
include/stp/_compiler.h-30-#else
include/stp/_compiler.h:31:# define stp_unreachable()  __builtin_unreachable()
include/stp/_compiler.h-32-#endif


I'd do that for glibc, but I don't see any facility.  Maybe we should add an 
__impossible() macro to document UB, and help the compiler.

Cheers,

Alex

>>       return NULL;
>>     if (dst == NULL)
>>       return NULL;
>>     dsize = end - dst;
>>     trunc = (slen >= dsize);
>>     dlen = trunc ? dsize - 1 : slen;
>>     dst[dlen] = 0;
>>     return mempcpy(dst, src, dlen) + trunc;
>> }
>>
>> This adds a '+' operation, so the difference compared to your strlcpy(3) is
>> smaller, but stpecpy() still wins, I think.
>>
>>
>> --
>> <http://www.alejandro-colomar.es/>

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]