Re: [PATCH] drm: move allocation out of drm_get_format_name()

[Christian KÃnig]

Am 05.11.2016 um 17:49 schrieb Rob Clark:
> On Sat, Nov 5, 2016 at 12:38 PM, Eric Engestrom <eric@xxxxxxxxxxxx> wrote:
> > On Saturday, 2016-11-05 13:11:36 +0100, Christian KÃnig wrote:
> > > Am 05.11.2016 um 02:33 schrieb Eric Engestrom:
> > > > +typedef char drm_format_name_buf[32];
> > > Please don't use a typedef for this, just define the maximum size of
> > > characters the function might write somewhere.
> > >
> > > See the kernel coding style as well:
> > > > In general, a pointer, or a struct that has elements that can reasonably
> > > > be directly accessed should **never** be a typedef.
> > I would normally agree as I tend to hate typedefs ($DAYJOB {ab,mis}uses
> > them way too much), and your way was what I wrote at first, but Rob Clark's
> > typedef idea makes it much harder for someone to allocate a buffer of
> > the wrong size, which IMO is good thing here.
> IMHO I would make a small test program to verify this actually helps
> the compiler catch problems.  And if it does, I would stick with it.
> The coding-style should be guidelines, not something that supersedes
> common sense / practicality.

Well completely agree that we should be able to question the coding 
style rules, but when we do it we discuss this on a the mailing list 
first and then start to use it in code. Not the other way around.

> That is my $0.02 anyways.. if others vehemently disagree and want to
> dogmatically stick to the coding-style guidelines, ok then.  OTOH, if
> this approach doesn't help the compiler catch issues, then it isn't
> worth it.

Yeah, exactly that's the point. If I'm not completely mistaken the 
compiler won't issue a warning here if you pass an array with the wrong 
size.

I think you need something like "struct drm_format_name_buf { char 
str[32]; };" to trigger this.

Apart from that is this function really called so often that using 
kasprintf() is a problem here? Or is there another motivation behind the 
change?

Regards,
Christian.

> BR,
> -R
>
> > I can rewrite the typedef out if you think it's better.
> >
> > Cheers,
> >    Eric
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel