[Mesa-dev] [PATCH 03/13] isl: Don't use designated initializers in the header

Jason Ekstrand jason at jlekstrand.net
Tue Apr 19 23:13:06 UTC 2016


On Tue, Apr 19, 2016 at 3:18 PM, Chad Versace <chad.versace at intel.com>
wrote:

> On Tue 19 Apr 2016, Emil Velikov wrote:
> > On 16 April 2016 at 20:45, Jason Ekstrand <jason at jlekstrand.net> wrote:
> > > C++ doesn't support designated initializers and g++ in particular
> doesn't
> > > handle them when the struct gets complicated, i.e. has a union.
> > > ---
> > >  src/intel/isl/isl.h | 32 ++++++++++++++++++++------------
> > >  1 file changed, 20 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
> > > index 33d43d7..c3a1106 100644
> > > --- a/src/intel/isl/isl.h
> > > +++ b/src/intel/isl/isl.h
> > > @@ -988,25 +988,35 @@ isl_surf_info_is_z32_float(const struct
> isl_surf_init_info *info)
> > >  static inline struct isl_extent2d
> > >  isl_extent2d(uint32_t width, uint32_t height)
> > >  {
> > > -   return (struct isl_extent2d) { .w = width, .h = height };
> > > +   struct isl_extent2d e = {
> > > +      { width, },
> > > +      { height, },
> > > +   };
> > > +   return e;
> > >  }
> > >
> > Please use something like the following
> >
> >    struct isl_extent2d e;
> >
> >    memset(&e, 0, sizeof(e));
> >    e.w = width;
> >    e.h = height;
> >
> >    return e;
> >
> > It is a bit over expressive to write/look at, although it will give
> > you exactly what you want _all_ the time.
> > Otherwise you risk feeding garbage and/or setting the wrong struct
> > member as isl_extent2d change.
> >
> > Hunting bugs that this may cause, is not what you want to do. Plus the
> > compiler will optimise it to the exact same code.
>
> I agree with Emil. Let's use explicit member names when initializing the
> structs. But, the memset isn't needed. The below initializaing pattern
> is more concise and *safer*, as it doesn't requiring typing the struct
> size.
>
>         struct isl_extent2d e = {0};
>
>         e.w = width;
>         e.h = height;
>
>         return e;
>

Done
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160419/424c7c5c/attachment.html>


More information about the mesa-dev mailing list