On Mon, 12 Feb 2018 12:28:55 +0000 Daniel Stone <dan...@fooishbar.org> wrote:
> Hi,
>
> On 9 February 2018 at 13:07, Pekka Paalanen <ppaala...@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
pgpYo2qxYG6Ha.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
