[PATCH weston 7/9] compositor-drm: fix uninitialized bytes on modeinfo

[Pekka Paalanen]

On Mon, 12 Feb 2018 12:28:55 +0000
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 9 February 2018 at 13:07, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > index 1897f455..a975b379 100644
> > --- a/libweston/compositor-drm.c
> > +++ b/libweston/compositor-drm.c
> > @@ -4347,6 +4347,8 @@ parse_modeline(const char *s, drmModeModeInfo *mode)
> >         char vsync[16];
> >         float fclock;
> >
> > +       *mode = (drmModeModeInfo){};  
> 
> Second and last nitpick: this is very clever, but should probably just
> use memset() like everywhere else.

I hate memset. :-)

You have to explicitly give the size, and you have to remember in which
order the memset arguments should come. The above syntax may look a
little strange if you're not used to it, but I would much rather let
the compiler figure things out. We already use the same compiler
feature to initialize local struct variables, without the cast.

It certainly takes me several brain cycles to check a memset, e.g. this
line has two bugs:

	memset(mode, sizeof mode, 0);

Would you take a patch to replace all memsets with the above assignment
form?

I believe nothing is depending on the values of implicit padding bytes
in structs anyway, if such happen to exist and if compiler would happen
to leave them uninitialized.

Or if you insist on memset, how about a macro:

#define MEMCLEARPTR(x) memset((x), 0, sizeof *(x))


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180212/1a2b2f65/attachment-0001.sig>