Closed Bug 782111 Opened 12 years ago Closed 9 years ago

implicit declaration of pthread_set_name_np(), -Werror

Categories

(NSPR :: NSPR, defect, P2)

4.9.2
All
FreeBSD
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jbeich, Assigned: jbeich)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Attached patch fix warnings about pthread_set_name_np() (obsolete) (deleted) — Splinter Review
Attachment #651186 - Flags: review?(landry)
Attachment #651186 - Flags: review?(wtc)
Attached patch fix warnings about pthread_set_name_np(), v2 (obsolete) (deleted) — Splinter Review
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: 1243493
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
(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 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 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.
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
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.
(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.

Attachment

General

Creator:
Created:
Updated:
Size: