[Spice-devel] [vdagent-win PATCH v12 3/6] Write code to decode PNG format

[Christophe de Dinechin]

> On 26 Jul 2017, at 09:58, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
> 
> On 25 Jul 2017, at 20:01, Frediano Ziglio <fziglio at redhat.com <mailto:fziglio at redhat.com>> wrote:
> 
> 
> On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote:
> +size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data,
> size_t size)
> +{
> [...]
> +    const unsigned int width = png_get_image_width(png, info);
> +    const unsigned int height = png_get_image_height(png, info);
> +    const size_t stride = compute_dib_stride(width, out_bits);
> +    const size_t image_size = stride * height;
> +    const int palette_colors = [=]() {
> 
> If you want to use a function, then give it a name, if you don't want to
> use a function, then this can just go in convert_to_dib() body rather
> than through a lambda (I know the latter means removing the 'const’).
> 
> Removing the ‘const’? Why?
> 
> 
> Lambdas are functions.
> 
> I believe Christophe is objecting to using a lambda whereas all other
> computations use named functions. But frankly, here, I would have
> written:
> 
>     const int palette_colors =
>           out_bits > 8  ? 0
>         : bits == 2     ? 4
>         : 1 << out_bits;
> This was exactly the previous version, just my version has some additional comments
> to make more readable.

So what made you change it? Do you feel that the lambda is more readable somehow?

> Not that I don’t like lambdas, but here, it’s a lot of additional syntactic noise.
> 
> Out of curiosity, I submitted both variants of the code to g++, and got
> exactly the same generated code.
> 
> I personally think the purpose of that code was clear even without
> a name.
> Using a "standard" function requires to declare parameters and pass
> them manually, something that lambda capture can do automatically.
> 
> 
> Christophe
> 
> 
> By the way, got new version with more old style code.
> 
> Frediano
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170726/9eeec3d0/attachment-0001.html>