| Message ID | 5b141b336816fb546417579055a8edfe8f5cc0eb.1570482718.git.crobinso@redhat.com |
|---|---|
| State | Accepted |
| Commit | c87784be89343642bda6c3020362173b0936b2da |
| Headers | show |
| Series | storagefile, security: qcow2 data_file support | expand |
On 10/7/19 6:49 PM, Cole Robinson wrote: > To backingFormat, which makes it more clear. Move it to the end of > the argument list which will scale nicer with future patches > > Signed-off-by: Cole Robinson <crobinso@redhat.com> > --- This really makes it clearer. I was getting confused about whether 'format' was referring to the image or its backing file in these patches. > src/util/virstoragefile.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c > index d6bd38777b..8cd576a463 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -427,9 +427,9 @@ cowGetBackingStore(char **res, > > > static int > -qcow2GetExtensions(int *format, > - const char *buf, > - size_t buf_size) > +qcow2GetExtensions(const char *buf, > + size_t buf_size, > + int *backingFormat) > { > size_t offset; > size_t extension_start; > @@ -509,8 +509,8 @@ qcow2GetExtensions(int *format, > case QCOW2_HDR_EXTENSION_BACKING_FORMAT: > if (buf[offset+len] != '\0') > break; > - *format = virStorageFileFormatTypeFromString(buf+offset); > - if (*format <= VIR_STORAGE_FILE_NONE) > + *backingFormat = virStorageFileFormatTypeFromString(buf+offset); > + if (*backingFormat <= VIR_STORAGE_FILE_NONE) > return -1; > } > > @@ -561,7 +561,7 @@ qcowXGetBackingStore(char **res, > memcpy(*res, buf + offset, size); > (*res)[size] = '\0'; > > - if (qcow2GetExtensions(format, buf, buf_size) < 0) > + if (qcow2GetExtensions(buf, buf_size, format) < 0) > return BACKING_STORE_INVALID; Not sure if the next patches are changing it or making it obsolete, but you can change 'format' to 'backingFormat' inside qcowXGetBackingStore too. Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > return BACKING_STORE_OK; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 10/9/19 5:14 PM, Daniel Henrique Barboza wrote: > > > On 10/7/19 6:49 PM, Cole Robinson wrote: >> To backingFormat, which makes it more clear. Move it to the end of >> the argument list which will scale nicer with future patches >> >> Signed-off-by: Cole Robinson <crobinso@redhat.com> >> --- > > This really makes it clearer. I was getting confused about whether > 'format' was referring to the image or its backing file in these > patches. > > > >> src/util/virstoragefile.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c >> index d6bd38777b..8cd576a463 100644 >> --- a/src/util/virstoragefile.c >> +++ b/src/util/virstoragefile.c >> @@ -427,9 +427,9 @@ cowGetBackingStore(char **res, >> static int >> -qcow2GetExtensions(int *format, >> - const char *buf, >> - size_t buf_size) >> +qcow2GetExtensions(const char *buf, >> + size_t buf_size, >> + int *backingFormat) >> { >> size_t offset; >> size_t extension_start; >> @@ -509,8 +509,8 @@ qcow2GetExtensions(int *format, >> case QCOW2_HDR_EXTENSION_BACKING_FORMAT: >> if (buf[offset+len] != '\0') >> break; >> - *format = virStorageFileFormatTypeFromString(buf+offset); >> - if (*format <= VIR_STORAGE_FILE_NONE) >> + *backingFormat = >> virStorageFileFormatTypeFromString(buf+offset); >> + if (*backingFormat <= VIR_STORAGE_FILE_NONE) >> return -1; >> } >> @@ -561,7 +561,7 @@ qcowXGetBackingStore(char **res, >> memcpy(*res, buf + offset, size); >> (*res)[size] = '\0'; >> - if (qcow2GetExtensions(format, buf, buf_size) < 0) >> + if (qcow2GetExtensions(buf, buf_size, format) < 0) >> return BACKING_STORE_INVALID; > > Not sure if the next patches are changing it or making it obsolete, but > you can > change 'format' to 'backingFormat' inside qcowXGetBackingStore too. > Later patches don't change it, as you've seen. We could change it, but context wise it's less necessary there IMO, and also we should then change it for the other GetBackingStore functions as well. Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d6bd38777b..8cd576a463 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -427,9 +427,9 @@ cowGetBackingStore(char **res, static int -qcow2GetExtensions(int *format, - const char *buf, - size_t buf_size) +qcow2GetExtensions(const char *buf, + size_t buf_size, + int *backingFormat) { size_t offset; size_t extension_start; @@ -509,8 +509,8 @@ qcow2GetExtensions(int *format, case QCOW2_HDR_EXTENSION_BACKING_FORMAT: if (buf[offset+len] != '\0') break; - *format = virStorageFileFormatTypeFromString(buf+offset); - if (*format <= VIR_STORAGE_FILE_NONE) + *backingFormat = virStorageFileFormatTypeFromString(buf+offset); + if (*backingFormat <= VIR_STORAGE_FILE_NONE) return -1; } @@ -561,7 +561,7 @@ qcowXGetBackingStore(char **res, memcpy(*res, buf + offset, size); (*res)[size] = '\0'; - if (qcow2GetExtensions(format, buf, buf_size) < 0) + if (qcow2GetExtensions(buf, buf_size, format) < 0) return BACKING_STORE_INVALID; return BACKING_STORE_OK;
To backingFormat, which makes it more clear. Move it to the end of the argument list which will scale nicer with future patches Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/virstoragefile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list