search

bbc / bmx

Library and utilities to read and write broadcasting media files. Primarily supports the MXF file format
BSD 3-Clause "New" or "Revised" License
54 stars 16 forks source link

strdup implicitly defined when building libMXF using -std=c11 #77

Open ErikCJohansson opened 1 month ago

ErikCJohansson commented 1 month ago

When running my app on Rocky Linux 8.10 I got a crash in mxf_free_data_model() via the ClipWriter destructor, which I tracked down to the pointer returned by strdup() in register_set_def() in mxf_data_model.c getting trashed.

Turns out that when building using gcc (tried 9.2.1 & 12.2.1) and -std=c11, strdup() is not defined unless defining e.g. _POSIX_C_SOURCE >= 200809L before including string.h (man strdup). Without this you'll end up with the implicit declaration which returns 32-bit int. This all works fine until you start getting heap addresses that don't fit in 32-bits.

To reproduce, I build libMXF like this: CFLAGS="-std=c11 " cmake bmx/deps/libMXF/ -DLIBMXF_BUILD_TESTING=OFF -DCMAKE_BUILD_TYPE=Debug -DCMAKE_VERBOSE_MAKEFILE=ON

Then built and ran my sample app (source attached): gcc bmxheap.c -ggdb -o bmxheap -I bmx/deps/libMXF/ -L ./mxf -lMXF bmxheap.c.gz

LD_LIBRARY_PATH=./mxf:$LD_LIBRARY_PATH ./bmxheap

philipnbbc commented 1 month ago

Thanks for your report.

I had a quick look and managed to reproduce it using your steps. I also note that there a couple other implicitly declared functions, namely gmtime_r, truncate and fileno. If gnu11 (c11 plus GNU extension) is used instead then there is no issue, which I guess is similar to what happens when you define _POSIX_C_SOURCE >= 200809L. So as it stands libMXF has some assumptions that are not supported in c11. I'm not sure if there is an easy fix but will take another look probably next week.

ErikCJohansson commented 1 month ago

May not require a fix in libMXF, just a thing to be aware of. Had it been among the issues when I started looking it would've saved me some time. Now it's there :-)

philipnbbc commented 3 weeks ago

I went with https://github.com/bbc/bmx/pull/78 which makes the warning an error. Support for implicit functions had been removed in C99 and in the context of bmx there is no need to allow them. It's not worthwhile implementing alternatives for strdup, fileno etc. as these (POSIX) extensions should be available.