On Thursday, 2018-12-20 08:30:53 +0000, Silvestrs Timofejevs wrote:
> Eric, thank you for reviewing my patch. I have expanded little bit
> on some of your comments bellow. I will be sending fallow up V2 of the patch
> shortly.
> On Thu, Dec 06, 2018 at 11:00:25AM +0000, Eric Engestrom wrote:
[...]
> > STATIC_ASSERT(sizeof(attr.surfString) <= sizeof("win,pb,pix,str,prsv"));
> I guess you meant ">="?
Indeed ^^'
[...]
> > > +/**
> > > + * Print the list of configs and the associated attributes.
> > > + */
> > > +void eglPrintConfigDebug(_EGLDriver *const drv, _EGLDisplay *const dpy,
> > > + EGLConfig *const configs, const EGLint
> > > numConfigs,
> > > + const enum EGL_CONFIG_DEBUG_OPTION printOption);
> >
> > `const` on non-pointers (ie. all the `const` here) in prototypes has
> > no effect, and I'm pretty sure the compiler will print a warning about it.
> >
> > Thanks for thinking about using `const` on pointers though :)
> > Please move them to the left of the `*` so that they can const the data
> > they point to ;)
> The reason why I didn't make data "const" is because eventually these
> parameters are passed into the existing MESA EGL functions outside of this
> change. The parameters for those functions are not constified - so it will
> result in a compiler warning that const is dropped. And I think it makes more
> sense to make them non-const from the beginning rather than cast the const
> away
> further down the pipe, as it is misleading.
That's a good point, I didn't realise we were lacking so many `const`
throughout src/egl/. I'll add them at some point, and I guess I'll add
them to your functions at the same time.
Point still stands about the compiler warning for the const in the
prototype above though ;)
Try compiling your code with a recent version of gcc (8 or above) and
clang, there might be other warnings as well.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
