| Summary: | media-libs/nas: should not define abs() as macros | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Anthony Basile <blueness> |
| Component: | [OLD] Library | Assignee: | Gentoo Sound Team <sound> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | vapier, vitto.giova |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | http://radscan.com/pipermail/nas/2015-April/001331.html | ||
| See Also: | https://bugs.gentoo.org/show_bug.cgi?id=510770 | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Attachments: | Patch to remove abs() and fabs() | ||
Created attachment 402194 [details, diff]
Patch to remove abs() and fabs()
Upstream looks inactive since 2013. Also, it should be safe to just remove abs() and fabs() since they are defined in POSIX. This patch should do the trick.
Patch sent upstream at ${URL}.
(In reply to Anthony Basile from comment #1) > Created attachment 402194 [details, diff] [details, diff] > Patch to remove abs() and fabs() > > Upstream looks inactive since 2013. Also, it should be safe to just remove > abs() and fabs() since they are defined in POSIX. This patch should do the > trick. I tested in on uclibc, glibc an musl and it just works, as expected. (In reply to Anthony Basile from comment #3) > (In reply to Anthony Basile from comment #1) > > Created attachment 402194 [details, diff] [details, diff] [details, diff] > > Patch to remove abs() and fabs() > > > > Upstream looks inactive since 2013. Also, it should be safe to just remove > > abs() and fabs() since they are defined in POSIX. This patch should do the > > trick. > > I tested in on uclibc, glibc an musl and it just works, as expected. This patch was accepted upstream. Permission to backport? http://radscan.com/pipermail/nas/2015-April/thread.html#1331 (In reply to Anthony Basile from comment #4) > (In reply to Anthony Basile from comment #3) > > (In reply to Anthony Basile from comment #1) > > > Created attachment 402194 [details, diff] [details, diff] [details, diff] [details, diff] > > > Patch to remove abs() and fabs() > > > > > > Upstream looks inactive since 2013. Also, it should be safe to just remove > > > abs() and fabs() since they are defined in POSIX. This patch should do the > > > trick. > > > > I tested in on uclibc, glibc an musl and it just works, as expected. > > This patch was accepted upstream. Permission to backport? > > http://radscan.com/pipermail/nas/2015-April/thread.html#1331 Committed with Chainsaw's permission in IRC. |
In nas-1.9.4, we have the following in server/include/misc.h: #ifndef abs #define abs(a) ((a) > 0 ? (a) : -(a)) #endif #ifndef fabs #define fabs(a) ((a) > 0.0 ? (a) : -(a)) /* floating absolute value */ #endif This is done to make sure abs() and fabs() are available, and makes the mistaken assumption that these are defined as macros. But abs() and fabs() are a POSIX *functions* prototyped in stdlib.h and math.h respectively (man 3 abs/fabs). If your libc is provided by glibc (or musl) then you'll get away with this, but on a uclibc system using the latest git/HEAD, or the latest 0.9.33 branch, but not the latest release 0.9.33.2, ie after commit uclibc's commit 2e2dc998d8a6d9d8877524564f5f1136c608e859, you'll get into trouble because those macros in misc.h can get defined *before* stdlib.h is included (or before math.h in the case of fabs). This happens in nas's server/os/access.c which includes "misc.h" before <X11/Xauth.h> which indirectly pulls in stdlib.h through bits/sched.h. And boom! gcc -c -O2 -fno-strength-reduce -fno-strict-aliasing -I. -I../include -I../../include -I../../include -I/usr/include -Dlinux -D__amd64__ -D_POSIX_SOURCE -D_POSIX_C_SOURCE=2 -D_BSD_SOURCE -D_SVID_SOURCE -DX_LOCALE -DFUNCPROTO=15 -DNARROWPROTO -DUNIXCONN -DTCPCONN access.c In file included from /usr/include/bits/sched.h:112:0, from /usr/include/sched.h:35, from /usr/include/pthread.h:25, from /usr/include/bits/uClibc_mutex.h:15, from /usr/include/bits/uClibc_stdio.h:107, from /usr/include/stdio.h:72, from /usr/include/X11/Xauth.h:59, from access.c:60: /usr/include/stdlib.h:713:12: error: expected identifier or '(' before 'int' /usr/include/stdlib.h:713:12: error: expected ')' before '>' token make[2]: *** [access.o] Error 1 make[2]: Leaving directory `/var/tmp/portage/media-libs/nas-1.9.4/work/nas-1.9.4-default/server/os' make[1]: *** [os] Error 2 make[1]: Leaving directory `/var/tmp/portage/media-libs/nas-1.9.4/work/nas-1.9.4-default/server' make: *** [all] Error 2 Unfortunately nas uses Imakefile/Makefile and not autotools, so there's no easy way to check for the availability of abs/fabs. Fortunately, you can probably just patch out those macros and be safe on all posix compliant systems. Note: this bug has enough blame to go around. Including stdlib.h in bits/sched.h is questionable and something that I'll bring up with uclibc upstream. Reproducible: Always