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

Fix compiler error; potential NULL arg in %s format string #644

Closed waveform80 closed 4 years ago

waveform80 commented 4 years ago

While building userland for the Ubuntu packaging under gcc-10, I encountered the following error:

In file included from /<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/build/inc/interface/vcos/vcos.h:144,
                 from /<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/interface/vmcs_host/linux/vcfilesys.c:56:
/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/interface/vmcs_host/linux/vcfilesys.c: In function ‘vc_hostfs_totalspace64’:
/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/build/inc/interface/vcos/vcos_logging.h:234:88: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
  234 | #  define _VCOS_LOG_X(cat, _level, fmt...)   do { if (vcos_is_log_enabled(cat,_level)) vcos_log_impl(cat,_level,fmt); } while (0)
      |                                                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/build/inc/interface/vcos/vcos_logging.h:245:32: note: in expansion of macro ‘_VCOS_LOG_X’
  245 | # define vcos_log_info(...)    _VCOS_LOG_X(VCOS_LOG_CATEGORY, VCOS_LOG_INFO, __VA_ARGS__)
      |                                ^~~~~~~~~~~
/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/interface/vmcs_host/linux/vcfilesys.c:79:26: note: in expansion of macro ‘vcos_log_info’
   79 | #define DEBUG_MINOR(...) vcos_log_info(__VA_ARGS__)
      |                          ^~~~~~~~~~~~~
/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/interface/vmcs_host/linux/vcfilesys.c:1019:4: note: in expansion of macro ‘DEBUG_MINOR’
 1019 |    DEBUG_MINOR( "vc_hostfs_totalspace for '%s' returning %" PRId64 "", path, ret );
      |    ^~~~~~~~~~~
/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/interface/vmcs_host/linux/vcfilesys.c:1019:44: note: format string is defined here
 1019 |    DEBUG_MINOR( "vc_hostfs_totalspace for '%s' returning %" PRId64 "", path, ret );
      |                                            ^~
cc1: all warnings being treated as errors
make[3]: *** [host_applications/linux/libs/bcm_host/CMakeFiles/bcm_host.dir/build.make:79: host_applications/linux/libs/bcm_host/CMakeFiles/bcm_host.dir/__/__/__/__/interface/vmcs_host/linux/vcfilesys.c.o] Error 1
make[3]: Leaving directory '/<<BUILDDIR>>/raspberrypi-userland-0~20200520+git2fe4ca3/obj-arm-linux-gnueabihf'
make[2]: *** [CMakeFiles/Makefile2:3390: host_applications/linux/libs/bcm_host/CMakeFiles/bcm_host.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....

This patch simply moves the DEBUG_MINOR call into the if(path) block to avoid this possibility, though it does mean that in the case of an out-of-memory condition (where strdup has returned NULL) that nothing is output by DEBUG_MINOR. I'm happy to re-jig this to call DEBUG_MINOR in both cases (e.g. with an alternate output message when path is NULL) if required.

6by9 commented 4 years ago

Hi @waveform80 This overlaps with #631 and #635 .

I'm not convinced by the gcc change, but that is what it is. The original behaviour of printing "string is (null)" is far more useful than a bundle of build warnings.

I want to confirm, but I'm fairly certain that vc_filesys is totally unused (I think it's part of the VPU accessing the host filing system), so we're better off just killing it.

pelwell commented 4 years ago

The vc_hostfs code (which is Linux user-space support for file accesses from the VPU) hasn't been used on a Pi in my time here, and possible never.

6by9 commented 4 years ago

The vc_hostfs code (which is Linux user-space support for file accesses from the VPU) hasn't been used on a Pi in my time here, and possible never.

Thanks for the confirmation - that was my understanding too. Time for a nuke then.

waveform80 commented 4 years ago

The vc_hostfs code (which is Linux user-space support for file accesses from the VPU) hasn't been used on a Pi in my time here, and possible never.

Thanks for the confirmation - that was my understanding too. Time for a nuke then.

Even better :) Less code is always nice!