On Mon, 12 Feb 2018 12:59:22 +0000 Daniel Stone <dan...@fooishbar.org> wrote:
> Hi,
>
> On 12 February 2018 at 12:51, Pekka Paalanen <ppaala...@gmail.com> wrote:
> > On Mon, 12 Feb 2018 12:28:55 +0000 Daniel Stone <dan...@fooishbar.org>
> > wrote:
> >> Second and last nitpick: this is very clever, but should probably just
> >> use memset() like everywhere else.
> >
> > I hate memset. :-)
>
> Me too!
>
> > 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);
>
> To be fair, GCC does warn about both of those, and has done since 4.x IIRC.
>
> foo.c: In function ‘main’:
> foo.c:17:2: warning: ‘memset’ used with constant zero length
> parameter; this could be due to transposed parameters
> [-Wmemset-transposed-args]
> memset(bar, sizeof bar, 0);
>
> foo.c: In function ‘main’:
> foo.c:17:24: warning: argument to ‘sizeof’ in ‘memset’ call is the
> same expression as the destination; did you mean to dereference it?
> [-Wsizeof-pointer-memaccess]
> memset(bar, 0, sizeof bar);
Oh, that's nice.
> > 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.
>
> Hmm. On the other hand, if the ={} form only zeroes bytes accessible
> via members, might this caveat not defeat the purpose of the patch?
> IOW, if there is implicit padding not cleared by this assignment form,
> but something is copying the entire size of the struct, that may be
> treated as a read from uninitialised memory.
True. The same caveat applies to anything we initialize without a full
memset. I'd think it to be so common though that tools like Valgrind
might have special-casing there.
> > Or if you insist on memset, how about a macro:
> >
> > #define MEMCLEARPTR(x) memset((x), 0, sizeof *(x))
>
> I'd definitely be happy with that.
Even though I said it, I don't like it too much.
> I don't really mind which form we take, as long as it a) works and b)
> is consistent. Being harder to get wrong than raw memset is definitely
> a feature too. :)
Yeah, I'll go with memset for now, since the compiler warnings.
Thanks,
pq
pgpY6xlM1Ee_O.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
