Messages in this thread | | | Date | Wed, 1 Apr 2020 18:13:32 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH 1/1] lib/vsprintf: Add support for printing V4L2 and DRM fourccs |
| |
On Wed, Apr 01, 2020 at 04:13:51PM +0200, Hans Verkuil wrote: > On 4/1/20 4:05 PM, Sakari Ailus wrote: > > Add a printk modifier %ppf (for pixel format) for printing V4L2 and DRM > > pixel formats denoted by 4ccs. The 4cc encoding is the same for both so > > the same implementation can be used.
%p4cc ?
> > + char ch[2] = { 0 }; > > This can just be '{ };'
The latter is GCC extension, while above is C standard. Former is slightly better I think. Though see below.
> > + unsigned int i; > > + > > + if (check_pointer(&buf, end, fourcc, spec)) > > + return buf; > > + > > + switch (fmt[1]) { > > + case 'f':
> > + for (i = 0; i < sizeof(*fourcc); i++) { > > + ch[0] = *fourcc >> (i << 3); > > You need to AND with 0x7f, otherwise a big endian fourcc (bit 31 is set) > will look wrong. Also, each character is standard 7 bit ascii, bit 7 isn't > used except to indicate a BE variant.
Why not to do it once by a flag and do reset it once?
u32 tmp = *fourcc; bool be4cc = tmp & BIT(31);
tmp &= BIT(31);
On top of that, as promised above, why not simple do it in a simpler way, i.e. using standard idiom:
for (i = 0; i < sizeof(*fourcc); i++) { if (buf < end) *buf = tmp >> (i * 8); buf++; } ?
> > + buf = string(buf, end, ch, spec); > > + } > > + > > + if (*fourcc & BIT(31)) > > + buf = string(buf, end, "-BE", spec);
Another possibility
u8 ch[8];
if (*fourcc & BIT(31)) { put_unaligned_be32(tmp, &ch[0]); strcpy(&ch[4], "-BE"); } else { put_unaligned_le32(tmp, &ch[0]); strcpy(&ch[4], "-LE"); } return string(buf, end, &ch[0], spec);
> > + return buf; > > + default: > > + return error_string(buf, end, "(%pp?)", spec); > > + } > > +}
-- With Best Regards, Andy Shevchenko
| |