Re: [Xen-devel] [PATCH v5 06/10] xen: introduce a generic framebuffer driver

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.01.13 at 19:16, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 
>>> wrote:
> +int __init lfb_init(struct lfb_prop *lfbp)
> +{
> +    if ( lfbp->width > MAX_XRES || lfbp->height > MAX_YRES )
> +    {
> +        printk(XENLOG_WARNING "Couldn't initialize a %ux%u framebuffer 
> early.\n",
> +               lfbp->width, lfbp->height);
> +        return -EINVAL;
> +    }
> +
> +    lfb.lfbp = *lfbp;
> +    lfb.lbuf = NULL;
> +    lfb.text_buf = NULL;
> +    lfb.line_len = NULL;
> +
> +    lfb.lbuf = xmalloc_bytes(lfb.lfbp.bytes_per_line);
> +    if ( !lfb.lbuf )
> +        goto fail;
> +
> +    lfb.text_buf = xzalloc_bytes(lfb.lfbp.text_columns * lfb.lfbp.text_rows);
> +    if ( !lfb.text_buf )
> +        goto fail;
> +
> +    lfb.line_len = xzalloc_array(unsigned int, lfb.lfbp.text_columns);
> +    if ( !lfb.line_len )
> +        goto fail;

While minor, this is inefficient (and needlessly growing the source
size): The initialization to NULL above could be dropped, the allocs
all done in a row, and the results could be checked in one go.

> +
> +    return 0;
> +
> +fail:
> +    printk(XENLOG_ERR "Couldn't allocate enough memory to drive the 
> framebuffer\n");
> +    lfb_free();
> +
> +    return -ENOMEM;
> +}

Irrespective of the comment above, but provided there's no hidden
change in the code that got moved around, feel free to stick my ack
on it.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel