On 2013-11-29 18:51, Michal Nazarewicz wrote:
> From: Michal Nazarewicz <mina86@xxxxxxxxxx>
>
> load_waveform function checks whether padding bytes in stuff2a
> and stuff2b are all zero, but does so by treating those arrays
> as a single longer array. Since the structure is packed, and
> the size sum matches, it all works, but creates some confusion
> in the code.
>
> This commit changes the stuff2a and stuff2b arrays into pad1 and
> pad2 fields such that they cover the same bytes as the arrays
> covered, and changes the check in the load_waveform function so
> that the fields are read instead of iterating over an arary.
>
> It also renames the other “stuff” fields to “ignore*” fields to
> give them more semantic meaning.
>
> Signed-off-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
> ---
> drivers/video/metronomefb.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
> index 195cc2d..4f36a2b 100644
> --- a/drivers/video/metronomefb.c
> +++ b/drivers/video/metronomefb.c
> @@ -126,7 +126,7 @@ static struct fb_var_screeninfo metronomefb_var = {
>
> /* the waveform structure that is coming from userspace firmware */
> struct waveform_hdr {
> - u8 stuff[32];
> + u8 ignore1[32];
>
> u8 wmta[3];
> u8 fvsn;
> @@ -134,13 +134,14 @@ struct waveform_hdr {
> u8 luts;
> u8 mc;
> u8 trc;
> - u8 stuff3;
> + u8 ignore2;
>
> u8 endb;
> u8 swtb;
> - u8 stuff2a[2];
> + u32 pad1; /* u16 halfof(pad1) */
>
> - u8 stuff2b[3];
> + /* u16 halfof(pad1) */
> + u8 pad2;
I don't quite follow what those comments mean...
I know nothing of the hw in question, but I guess there's some reason
the stuff2a and stuff2b has been separate, and there's even a blank line
between. If they pad a particular block in the data structure, doesn't
your change make it confusing?
It wouldn't be too pad to have those 5 fields as separate ones. One u16
for the stuff2a, and u16 + u8 for the stuff2b. It'd be simple to check
that those are zero.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
