raspberrypi / userland

Source code for ARM side libraries for interfacing to Raspberry Pi GPU.
BSD 3-Clause "New" or "Revised" License
2.05k stars 1.09k forks source link

warning: implicit declaration of function 'sbrk' and 'strdup' #316

Open fordfrog opened 8 years ago

fordfrog commented 8 years ago

while installing latest (20160531) raspberrypi-userland on gentoo, i got these QA warnings:

 * QA Notice: Package triggers severe warnings which indicate that it
 *            may exhibit random runtime failures.
 * /var/tmp/portage/media-libs/raspberrypi-userland-0_pre20160531/work/raspberrypi-userland-ed59ee1/interface/vcos/pthreads/vcos_platform.h:777:4: warning: implicit declaration of function ‘sbrk’ [-Wimplicit-function-declaration]
 * /var/tmp/portage/media-libs/raspberrypi-userland-0_pre20160531/work/raspberrypi-userland-ed59ee1/interface/vcos/pthreads/vcos_platform.h:818:4: warning: implicit declaration of function ‘strdup’ [-Wimplicit-function-declaration]

i don't know whether it is intended or it should be fixed so can someone who knows this stuff please clarify it?

fordfrog commented 8 years ago

i found this info: http://stackoverflow.com/questions/8440816/warning-implicit-declaration-of-function

so it means that these two functions are used before they are declared (either in header file or before they are first used).

popcornmix commented 8 years ago

sbrk should be declared in <unistd.h> which is being included. strdup should be declared in <string.h> which is being included. I'm not sure if the gentoo compiler doesn't support these?

I don't think sbrk or strdup are typically used by userland (mostly for test functions), and the lack of a declaration for a function with simple types is normally not fatal, so it's probably not a big issue.

@6by9 @pelwell any thoughts?

pelwell commented 8 years ago

strdup is part of POSIX so ought to be included. sbrk has been removed, so perhaps that failure is more understandable. Which compiler (including version) are you using?

ED6E0F17 commented 8 years ago

I have seen this error previously, and now that I have checked "string,h" on wheezy I understand why.

ED6E0F17 commented 8 years ago

It look as though "_GNU_SOURCE" needs to be defined to get "USE_XOPEN_EXTENDED", and then define sbrk and strdup. "_GNU_SOURCE" is defined in some places, but not everywhere that it is needed.

fordfrog commented 8 years ago

@pelwell gcc 4.9.3

ED6E0F17 commented 8 years ago

It becomes an issue when the compiler is throwing so many warnings that you stop reading them, and warning is given every time that "vcos.h" is included. Most of these seem to be in the "containers" directory, which defines POSIX level 2001, but strdup requires POSIX level 2008: https://github.com/raspberrypi/userland/blob/master/containers/CMakeLists.txt#L16

What I have done is this: https://github.com/ED6E0F17/userland/commit/cc3eb42f10ee3603649e586e1b196c1ae10f8a2e

popcornmix commented 8 years ago

@fordfrog does @ED6E0F17's commit work for you? If so, I'm happy to include a change like that.

ED6E0F17 commented 8 years ago

I need to abandon that patch. Cmake is not always used to build the applications in HelloPi, so it is necessary to keep the local definitions of _GNU_SOURCE there.

fordfrog commented 8 years ago

@popcornmix, @ED6E0F17: the patch works fine when building using cmake.

i will use it untill a more general solution is found. thank you.

luked99 commented 8 years ago

I think it might be possible to just remove the function that calls sbrk().

e.g.

https://github.com/luked99/userland-1/commit/627eaebe50a26e28be365aca03efb932a25228cf

ED6E0F17 commented 8 years ago

@luked99 : Didn`t you already fix this by sneaking a -D_GNU_SOURCE into your last pull request ?

luked99 commented 8 years ago

That pull request got abandoned in the end. I think just removing the code seems better. It was originally (IIRC) for the benefit of VideoCore test harnesses that wanted to check for memory leaks. But trying to do so that way just seems very optimistic (the VideoCore version of the same function is fine). Better to use Valgrind, and better to remove a function that is ill-defined and likely to confuse at best.

(In my opinion, obviously; feel free to disagree :-)

JamesH65 commented 6 years ago

After @luked99 merge last year, has this issue gone away? Seems related to #188

JamesH65 commented 6 years ago

I believe this has been fixed.

This issue will be closed within 30 days unless further interactions are posted. If you wish this issue to remain open, please add a comment. A closed issue may be reopened if requested.

aperezdc commented 6 years ago

FWIW, there is a related problem (missing definition of _GNU_SOURCE) for the calls to clock_gettime() which use the CLOCK_* macros. Code that uses them is present in vcos_platform.h, see https://github.com/Igalia/buildroot-wpe/pull/2#discussion_r224958314

My impression is that in general the headers assume that the compiler will be using -std=gnu<something> (for example -std=gnu99) and when a program is built with a plain -std=c<something> (like -std=c99) the compiler will complain.

There are two solutions I can think of: One is removing the usage of clock_gettime() from the header and put it into a .c file which gets built with the needed preprocessor symbol(s) defined; and the other is adding -D_GNU_SOURCE to the compiler flags listed in bcm_host.pc.in. The first option seems better to me, because it does not force every program which uses the userland drivers to be compiled with _GNU_SOURCE defined (and many may not need it, or even not build with it defined!).