Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed

Conversation

waveform80
Copy link
Contributor

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
Copy link
Contributor

6by9 commented Jul 27, 2020

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
Copy link
Contributor

pelwell commented Jul 27, 2020

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
Copy link
Contributor

6by9 commented Jul 27, 2020

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
Copy link
Contributor Author

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!

@waveform80 waveform80 closed this Jul 27, 2020
@waveform80 waveform80 deleted the fix-format-overflow branch July 28, 2020 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants