Re: pthread_setname_np API is bad

[Christos Zoulas <christos%zoulas.com@localhost>]

My worry is that someone will call pthread_setname_np() with a  "%thread%s" name argument and get a core dump on a NetBSD system since the string will be interpreted as a format (where in other OS's it will be taken literally and work.

christos

> On Aug 9, 2019, at 4:12 PM, Kamil Rytarowski <n54%gmx.com@localhost> wrote:
> 
> On 09.08.2019 14:42, Jaromír Doleček wrote:
>> I'd change this immediatelly using the normal rewrite logic in headers
>> we use for standard versioning. It would be really good to push this
>> into netbsd-9 too.
>> 
> 
> Ah right, we can do __rename() and do it now.
> 
>>>>> Personally, I find it convenient to use it like pthread_setname_np(t[i],
>>>>> "thread %d, i) and I would like to keep using it.
>>>> 
>>>> FWIW, I think that's a better suggestion than either of mine. I support that.
>>> 
>>> Do we have to wait for an API bump for this? Seems pretty harmless. Although
>>> it is probably better to have:
>>> 
>>>    int pthread_setname_np(pthread_t, const char *);
>>> and
>>>    int pthread_fmtname_np(pthread_t, const char * restrict format, ...)
>>>        __printflike(2, 3);
>>> 
>>> christos
>>> 
> 
> If we switch to pthread_setname_np() to fully API incompatible prototype
> than we will need to reiterate over existing users and add #ifdef for in
> my opinion too little gain.
> 
> I did some patching and notifying (this happened in MESA) of 3rd party
> code to support the NetBSD variation of pthread_setname_np(3) and I
> don't want to reiterate over all the users now.
> 
> My proposal is even (in practice) ABI compatible in a number of CPU ABIs
> (such as i386/amd64)... however too risky to tell whether it applies to
> all of them.
> 
> I think it is enough to keep a single function to set thread name, no
> need for two variations (setname and fmtname).
> 
> There are around 30 patched programs in pkgsrc tweaking
> pthread_setname_np() and a number of them with applied support upstream
> (150 total? there are 144 references in Debian repositories).
> 
> If someone will insist to use form pthread_setname_np(pthread_t, const
> char *), it will be allowed to do it with the ... function type, just
> pass full string in format and skip var args.
> 
> <sanitizer.log>