Closed
Bug 782111
Opened 12 years ago
Closed 9 years ago
implicit declaration of pthread_set_name_np(), -Werror
Categories
(NSPR :: NSPR, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
4.12
People
(Reporter: jbeich, Assigned: jbeich)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
wtc
:
review+
wtc
:
checked-in+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #651186 -
Flags: review?(landry)
Attachment #651186 -
Flags: review?(wtc)
It seems NetBSD needs to cast away const. In bug 752067 case the warning is actually an error. Reported by ryoon@netbsd.org.
I shouldn't have assumed the code from
pr/src/pthreads/ptthread.c:1647:42: warning: passing 'const char *' to
parameter of type 'void *' discards qualifiers [-Wincompatible-pointer-types]
pthread_setname_np(thread->id, "%s", name);
^~~~
pr/src/pthreads/ptthread.c:24:66: note: passing argument to parameter
'arg' here
int pthread_setname_np(pthread_t thread, const char *name, void *arg);
^
1 warning generated.
Attachment #651186 -
Attachment is obsolete: true
Attachment #651186 -
Flags: review?(wtc)
Attachment #651186 -
Flags: review?(landry)
Attachment #651629 -
Flags: review?(wtc)
Blocks: Wimplicit-function-declaration
Comment 3•9 years ago
|
||
Comment on attachment 651629 [details] [diff] [review]
fix warnings about pthread_set_name_np(), v2
Fwiw, i support this patch. Jan, just a detail, since pthread_set_name_np() returns an int on FreeBSD, are you sure you want to throw away the result ?
Wan-Teh, care to review/land this ?
Flags: needinfo?(wtc)
Attachment #651629 -
Flags: feedback+
(In reply to Landry Breuil (:gaston) from comment #3)
> since pthread_set_name_np() returns an int on FreeBSD
Did you mean NetBSD? The return value appears to be similar to Linux.
https://www.freebsd.org/cgi/man.cgi?query=pthread_set_name_np&sektion=3
http://netbsd.gw.com/cgi-bin/man-cgi?pthread_setname_np++NetBSD-current
http://man7.org/linux/man-pages/man3/pthread_setname_np.3.html
Comment 5•9 years ago
|
||
(In reply to Jan Beich from comment #4)
> (In reply to Landry Breuil (:gaston) from comment #3)
> > since pthread_set_name_np() returns an int on FreeBSD
>
> Did you mean NetBSD? The return value appears to be similar to Linux.
Hm, i supposed that since it was assigning the result it was returning something, didnt look at the API. I know on openbsd pthread_set_name_np is void. Disregard the comment then :)
NetBSD fix from comment 4. It still cannot fall through to Linux version because the 3rd cannot be omitted unlike "..." in printf().
Attachment #651629 -
Attachment is obsolete: true
Attachment #651629 -
Flags: review?(wtc)
Attachment #8713972 -
Flags: review?(wtc)
Comment 7•9 years ago
|
||
Comment on attachment 8713972 [details] [diff] [review]
fix warnings about pthread_set_name_np(), v2.1
Review of attachment 8713972 [details] [diff] [review]:
-----------------------------------------------------------------
r=wtc. Patch checked in to the NSPR trunk:
https://hg.mozilla.org/projects/nspr/rev/85f6c6480115
Attachment #8713972 -
Flags: review?(wtc)
Attachment #8713972 -
Flags: review+
Attachment #8713972 -
Flags: checked-in+
Comment 8•9 years ago
|
||
Comment on attachment 8713972 [details] [diff] [review]
fix warnings about pthread_set_name_np(), v2.1
Review of attachment 8713972 [details] [diff] [review]:
-----------------------------------------------------------------
::: pr/src/pthreads/ptthread.c
@@ +1755,5 @@
> return PR_FAILURE;
> memcpy(thread->name, name, nameLen + 1);
>
> +#if defined(OPENBSD) || defined(FREEBSD) || defined(DRAGONFLY)
> + pthread_set_name_np(thread->id, name);
Jan: could you please confirm that pthread_set_name_np returns void
on these three platforms? Comment 3 and comment 4 are confusing about
this issue. Thanks!
(In reply to Wan-Teh Chang from comment #8)
> could you please confirm that pthread_set_name_np returns void on these three platforms?
comment 1 version was based on respective manpages alone. comment 2 - after compiling under qemu. Feel free to check the prototypes yourself:
https://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/include/pthread_np.h
https://svnweb.freebsd.org/base/head/include/pthread_np.h?view=co
http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libpthread/include/pthread_np.h
http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/lib/libpthread/pthread.h
pthread_set_name_np() first appeared on FreeBSD circa 1998 when there was more than one implementation (1:N libc_r, M:N libkse, 1:1 libthr). Around the same year OpenBSD imported libc_r from FreeBSD which already had the function. And a few years later DragonFly was forked. While the code was refactored since then the prototype didn't change except for |const| in the 2nd argument.
NetBSD added pthread_setname_np() based on Tru64 in 2003 probably because it lacked pthread_set_name_np() to care about backward compatibility.
FYI, some non-GNU flavors of Linux have pthread_set_name_np() but return |int| instead of |void|: Xenomai and PTL.
Assignee | ||
Comment 10•9 years ago
|
||
Firefox uses pthread_set_name_np() in other places as well but only PosixNSPR.cpp checks return value.
https://dxr.mozilla.org/mozilla-central/search?q=pthread_set_name_np
Comment 11•9 years ago
|
||
There's a DragonFly patch in bug 884987, but I didn't land it because it's uploaded slightly oddly and I just haven't spent the time to get that sorted.
Comment 12•9 years ago
|
||
(In reply to Jan Beich from comment #9)
>
> comment 1 version was based on respective manpages alone. comment 2 - after
> compiling under qemu. Feel free to check the prototypes yourself:
> https://gitweb.dragonflybsd.org/dragonfly.git/blob_plain/HEAD:/include/
> pthread_np.h
> https://svnweb.freebsd.org/base/head/include/pthread_np.h?view=co
> http://cvsweb.openbsd.org/cgi-bin/cvsweb/~checkout~/src/lib/libpthread/
> include/pthread_np.h
> http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/lib/libpthread/pthread.h
Jan: Thanks a lot for the links. I verified the function prototypes on
these four BSDs match your code.
I think we can mark this bug fixed.
Assignee: wtc → jbeich
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Flags: needinfo?(wtc)
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 4.12
You need to log in
before you can comment on or make changes to this bug.
Description
•