| Message ID | 1253704471-30740-1-git-send-email-amit.shah@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
Amit Shah <amit.shah@redhat.com> writes: > If initializing a device fails, show the name of the device > and then exit "and then exit" is misleading, as you don't add an exit. > Signed-off-by: Amit Shah <amit.shah@redhat.com> > --- > hw/qdev.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/hw/qdev.c b/hw/qdev.c > index 1d79db0..62a6fc7 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -203,6 +203,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) > return NULL; > } > if (qdev_init(qdev) != 0) { > + qemu_error("Error initializing device %s\n", driver); > qdev_free(qdev); > return NULL; > }
Hey Markus, On (Wed) Sep 23 2009 [22:53:27], Markus Armbruster wrote: > Amit Shah <amit.shah@redhat.com> writes: > > > If initializing a device fails, show the name of the device > > and then exit > > "and then exit" is misleading, as you don't add an exit. The 'return NULL' does that -- the calling function exits. (doesn't the subject line clarify that?) > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > --- > > hw/qdev.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/hw/qdev.c b/hw/qdev.c > > index 1d79db0..62a6fc7 100644 > > --- a/hw/qdev.c > > +++ b/hw/qdev.c > > @@ -203,6 +203,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) > > return NULL; > > } > > if (qdev_init(qdev) != 0) { > > + qemu_error("Error initializing device %s\n", driver); > > qdev_free(qdev); > > return NULL; > > } Amit
Amit Shah <amit.shah@redhat.com> writes: > Hey Markus, > > On (Wed) Sep 23 2009 [22:53:27], Markus Armbruster wrote: >> Amit Shah <amit.shah@redhat.com> writes: >> >> > If initializing a device fails, show the name of the device >> > and then exit >> >> "and then exit" is misleading, as you don't add an exit. > > The 'return NULL' does that -- the calling function exits. > (doesn't the subject line clarify that?) You could say "show the name of the device before exiting". The message as you worded it made me look for an exit where there wasn't one before.
On (Thu) Sep 24 2009 [15:30:27], Markus Armbruster wrote: > Amit Shah <amit.shah@redhat.com> writes: > > > Hey Markus, > > > > On (Wed) Sep 23 2009 [22:53:27], Markus Armbruster wrote: > >> Amit Shah <amit.shah@redhat.com> writes: > >> > >> > If initializing a device fails, show the name of the device > >> > and then exit > >> > >> "and then exit" is misleading, as you don't add an exit. > > > > The 'return NULL' does that -- the calling function exits. > > (doesn't the subject line clarify that?) > > You could say "show the name of the device before exiting". That's what the subject says! :-) > The message as you worded it made me look for an exit where there wasn't > one before. I'll respin it nevertheless. Amit
Amit Shah <amit.shah@redhat.com> writes: > On (Thu) Sep 24 2009 [15:30:27], Markus Armbruster wrote: >> Amit Shah <amit.shah@redhat.com> writes: >> >> > Hey Markus, >> > >> > On (Wed) Sep 23 2009 [22:53:27], Markus Armbruster wrote: >> >> Amit Shah <amit.shah@redhat.com> writes: >> >> >> >> > If initializing a device fails, show the name of the device >> >> > and then exit >> >> >> >> "and then exit" is misleading, as you don't add an exit. >> > >> > The 'return NULL' does that -- the calling function exits. >> > (doesn't the subject line clarify that?) >> >> You could say "show the name of the device before exiting". > > That's what the subject says! :-) > >> The message as you worded it made me look for an exit where there wasn't >> one before. > > I'll respin it nevertheless. > > Amit Thanks, and hope you don't mind me being such a stickler for clear commit messages.
On (Thu) Sep 24 2009 [16:22:34], Markus Armbruster wrote: > > Thanks, and hope you don't mind me being such a stickler for clear > commit messages. As long as you widen your net -- the commit logs aren't all too good to read :-) Amit
Amit Shah <amit.shah@redhat.com> writes: > On (Thu) Sep 24 2009 [16:22:34], Markus Armbruster wrote: >> >> Thanks, and hope you don't mind me being such a stickler for clear >> commit messages. > > As long as you widen your net -- the commit logs aren't all too good to > read :-) It's an all too common complaint in my reviews. It stood out in your case not because the message was particularly bad, only because I couldn't find anything else to complain about :)
On 09/24/09 05:59, Amit Shah wrote: > Hey Markus, > > On (Wed) Sep 23 2009 [22:53:27], Markus Armbruster wrote: >> Amit Shah<amit.shah@redhat.com> writes: >> >>> If initializing a device fails, show the name of the device >>> and then exit >> >> "and then exit" is misleading, as you don't add an exit. > > The 'return NULL' does that -- the calling function exits. > (doesn't the subject line clarify that?) I have patches in the queue which will wind up this via device_add monitor command. Caller will *not* exit then ;) cheers, Gerd
diff --git a/hw/qdev.c b/hw/qdev.c index 1d79db0..62a6fc7 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -203,6 +203,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) return NULL; } if (qdev_init(qdev) != 0) { + qemu_error("Error initializing device %s\n", driver); qdev_free(qdev); return NULL; }
If initializing a device fails, show the name of the device and then exit Signed-off-by: Amit Shah <amit.shah@redhat.com> --- hw/qdev.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)