Code review comment for lp:~andreas-pokorny/mir/pixel-format-utils

> No major problems, but there are enough minor issues to justify a "Needs
> Fixing".
>
> (1) The copyright year is now wrong :)

Those years are updated when a file is touched?

> (2) Using the strongly typed MirPixelFormat, you don't have to check for
> invalid types. So:
> 100 + if (mir_pixel_format_invalid < format &&
> 101 + format < mir_pixel_formats)
> 102 + return 8;
> 103 + return 0;
> could just be:
> return 8;
> This is because it's already a precondition that "format" is a valid
> MirPixelFormat. The same goes for other functions. You never need to range-
> check a strongly typed enum.

But it is not really strongly typed - an unchecked static_cast is enough to turn an arbitrary integer into a meaningless pixel format. Additionally the enum itself provides invalid values too.

>
> (3) I assume the valid_format() function is for testing non-MirPixelFormat
> variables; integers? As such it might make sense for:
> bool valid_format(MirPixelFormat format)
> to be:
> bool valid_mir_pixel_format(int format)
> ?

At the point where it is used the integers transfered to the client have already been "converted". But the new name a a good idea/