[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] fbdev: add linux framebuffer display driver
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] fbdev: add linux framebuffer display driver. |
Date: |
Thu, 27 Jun 2013 09:23:25 -0400 |
On Thu, 27 Jun 2013 11:58:57 +0200
Gerd Hoffmann <address@hidden> wrote:
> Hi,
>
> >> + /* open framebuffer */
> >> + if (device == NULL) {
> >> + device = getenv("FRAMEBUFFER");
> >> + }
> >> + if (device == NULL) {
> >> + device = "/dev/fb0";
> >> + }
> >
> > Maybe this is a matter of taste, but I think that having this logic at
> > this layer makes the API harder to use. What about moving this to the
> > call in vl.c and making the device name required in QMP?
>
> I'll just drop the getenv. That will make "/dev/fb0" the default value
> no matter what. Ok?
I still prefer having no defaults in QMP (it's ok elsewhere, like HMP),
but I won't nack the patch of that.
> >> + s->fb = open(device, O_RDWR);
> >> + if (s->fb == -1) {
> >> + error_setg(err, "open %s: %s\n", device, strerror(errno));
> >
> > We have error_setg_errno() :)
>
> Also error_setg_file_open() ;)
Oh, I really forgot about it...