| Message ID | b004cd17d0fa02ed169c5b101fdbbb7fa2b6b633.1538652143.git.tgolembi@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qga: report serial number and disk node | expand |
Hi On Thu, Oct 4, 2018 at 3:22 PM Tomáš Golembiovský <tgolembi@redhat.com> wrote: > > It was not obvious what exactly the cryptic string copying does to the > GUID. This change makes the intent clearer. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > qga/commands-win32.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index d0d969d0ce..82881aa749 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -507,7 +507,14 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) > char dev_name[MAX_PATH]; > char *buffer = NULL; > GuestPCIAddress *pci = NULL; > - char *name = g_strdup(&guid[4]); > + char *name = NULL; > + > + if ((g_str_has_prefix(guid, "\\\\.\\") == TRUE) || > + (g_str_has_prefix(guid, "\\\\?\\") == TRUE)) { > + name = g_strdup(&guid[4]); I find "guid + 4" easier to read though > + } else { > + name = g_strdup(guid); > + } > > if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) { > error_setg_win32(errp, GetLastError(), "failed to get dos device name"); > -- > 2.19.0 >
+1, this is much clearer. On Thu, Oct 4, 2018 at 4:34 PM Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > Hi > > On Thu, Oct 4, 2018 at 3:22 PM Tomáš Golembiovský <tgolembi@redhat.com> wrote: > > > > It was not obvious what exactly the cryptic string copying does to the > > GUID. This change makes the intent clearer. > > > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > --- > > qga/commands-win32.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > > index d0d969d0ce..82881aa749 100644 > > --- a/qga/commands-win32.c > > +++ b/qga/commands-win32.c > > @@ -507,7 +507,14 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) > > char dev_name[MAX_PATH]; > > char *buffer = NULL; > > GuestPCIAddress *pci = NULL; > > - char *name = g_strdup(&guid[4]); > > + char *name = NULL; > > + > > + if ((g_str_has_prefix(guid, "\\\\.\\") == TRUE) || > > + (g_str_has_prefix(guid, "\\\\?\\") == TRUE)) { > > + name = g_strdup(&guid[4]); > > I find "guid + 4" easier to read though > > > + } else { > > + name = g_strdup(guid); > > + } > > > > if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) { > > error_setg_win32(errp, GetLastError(), "failed to get dos device name"); > > -- > > 2.19.0 > > >
On Thu, 4 Oct 2018 17:20:50 +0400 Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > Hi > > On Thu, Oct 4, 2018 at 3:22 PM Tomáš Golembiovský <tgolembi@redhat.com> wrote: > > > > It was not obvious what exactly the cryptic string copying does to the > > GUID. This change makes the intent clearer. > > > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > > --- > > qga/commands-win32.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > > index d0d969d0ce..82881aa749 100644 > > --- a/qga/commands-win32.c > > +++ b/qga/commands-win32.c > > @@ -507,7 +507,14 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) > > char dev_name[MAX_PATH]; > > char *buffer = NULL; > > GuestPCIAddress *pci = NULL; > > - char *name = g_strdup(&guid[4]); > > + char *name = NULL; > > + > > + if ((g_str_has_prefix(guid, "\\\\.\\") == TRUE) || > > + (g_str_has_prefix(guid, "\\\\?\\") == TRUE)) { > > + name = g_strdup(&guid[4]); > > I find "guid + 4" easier to read though I will change that, no problem. > > > + } else { > > + name = g_strdup(guid); > > + } > > > > if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) { > > error_setg_win32(errp, GetLastError(), "failed to get dos device name"); > > -- > > 2.19.0 > >
On 10/4/18 8:20 AM, Marc-André Lureau wrote: > Hi > > On Thu, Oct 4, 2018 at 3:22 PM Tomáš Golembiovský <tgolembi@redhat.com> wrote: >> >> It was not obvious what exactly the cryptic string copying does to the >> GUID. This change makes the intent clearer. In the subject line, s/striping/stripping/ (this is about performing a 'strip' operation on a prefix, but I read the subject as an instance of 'stripe' as in drawing a line or fragmenting data in a RAID). >> +++ b/qga/commands-win32.c >> @@ -507,7 +507,14 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) >> char dev_name[MAX_PATH]; >> char *buffer = NULL; >> GuestPCIAddress *pci = NULL; >> - char *name = g_strdup(&guid[4]); >> + char *name = NULL; >> + >> + if ((g_str_has_prefix(guid, "\\\\.\\") == TRUE) || >> + (g_str_has_prefix(guid, "\\\\?\\") == TRUE)) { I find that 'cond == true' is redundant to just writing 'cond'. And that sentiment applies to both the <stdbool.h> 'bool' and to the glib abomination TRUE (why they had to invent their own boolean names, worse in every way compared to <stdbool.h>, is beyond me). >> + name = g_strdup(&guid[4]); > > I find "guid + 4" easier to read though Concur.
diff --git a/qga/commands-win32.c b/qga/commands-win32.c index d0d969d0ce..82881aa749 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -507,7 +507,14 @@ static GuestPCIAddress *get_pci_info(char *guid, Error **errp) char dev_name[MAX_PATH]; char *buffer = NULL; GuestPCIAddress *pci = NULL; - char *name = g_strdup(&guid[4]); + char *name = NULL; + + if ((g_str_has_prefix(guid, "\\\\.\\") == TRUE) || + (g_str_has_prefix(guid, "\\\\?\\") == TRUE)) { + name = g_strdup(&guid[4]); + } else { + name = g_strdup(guid); + } if (!QueryDosDevice(name, dev_name, ARRAY_SIZE(dev_name))) { error_setg_win32(errp, GetLastError(), "failed to get dos device name");
It was not obvious what exactly the cryptic string copying does to the GUID. This change makes the intent clearer. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- qga/commands-win32.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)