[FFmpeg-devel] [PATCH V3 1/5] libavdevice/v4l2.c: fix build warning for [-Wformat-truncation=]

[Guo, Yejun]

> -----Original Message-----
> From: Nicolas George <george at nsup.org>
> Sent: 2021年2月26日 17:28
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Cc: Guo, Yejun <yejun.guo at intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH V3 1/5] libavdevice/v4l2.c: fix build
> warning for [-Wformat-truncation=]
> 
> Guo, Yejun (12021-02-26):
> > Here is the warning message:
> > src/libavdevice/v4l2.c: In function ‘v4l2_get_device_list’:
> > src/libavdevice/v4l2.c:1054:58: warning: ‘%s’ directive output may be
> truncated writing up to 255 bytes into a region of size 251
> [-Wformat-truncation=]
> >          snprintf(device_name, sizeof(device_name), "/dev/%s",
> entry->d_name);
> >                                                           ^~
> > src/libavdevice/v4l2.c:1054:9: note: ‘snprintf’ output between 6 and 261
> bytes into a destination of size 256
> >          snprintf(device_name, sizeof(device_name), "/dev/%s",
> entry->d_name);
> >
> >
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ~
> >
> > Signed-off-by: Guo, Yejun <yejun.guo at intel.com>
> > ---
> >  libavdevice/v4l2.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c index
> > 365bacd771..cb426cf2d5 100644
> > --- a/libavdevice/v4l2.c
> > +++ b/libavdevice/v4l2.c
> > @@ -1046,7 +1046,7 @@ static int v4l2_get_device_list(AVFormatContext
> *ctx, AVDeviceInfoList *device_l
> >          return ret;
> >      }
> >      while ((entry = readdir(dir))) {
> > -        char device_name[256];
> 
> > +        char device_name[sizeof(entry->d_name) + 5];
> 
> Unfortunately, that is not guaranteed to work: entry->d_name can be declared
> as char d_name[0] and the space dynamically allocated at the end of the
> structure.

'man readdir' on my system shows that it is d_name[256], see below.

           struct dirent {
               ino_t          d_ino;       /* Inode number */
               off_t          d_off;       /* Not an offset; see below */
               unsigned short d_reclen;    /* Length of this record */
               unsigned char  d_type;      /* Type of file; not supported
                                              by all filesystem types */
               char           d_name[256]; /* Null-terminated filename */
           };

If there is a possibility for 'char d_name[0]', yes, it is an issue in the patch.

> 
> I think a better course of action would be to acknowledge that this warning is
> a waste of time and disable it.

agree, and maybe we can just let this build warning there.

> 
> That does not mean we should not fix the cases where the buffer can be too
> small. We should, and we will find them without this warning, by making use
> of common sense. This instance is not one, because even with a billion
> webcams, /dev/video999999999 fits in 256 chars.
> 
> >
> >          if (!v4l2_is_v4l_dev(entry->d_name))
> >              continue;
> 
> Regards,
> 
> --
>   Nicolas George