| Message ID | 20210309102157.365356-2-david.edmondson@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | coroutine rwlock downgrade fix, minor VDI changes | expand |
On 3/9/21 11:21 AM, David Edmondson wrote: > If a new bitmap entry is allocated, requiring the entire block to be > written, avoiding leaking the buffer allocated for the block should > the write fail. > > Signed-off-by: David Edmondson <david.edmondson@oracle.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > block/vdi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/vdi.c b/block/vdi.c > index 5627e7d764..2a6dc26124 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -690,6 +690,7 @@ nonallocating_write: > > logout("finished data write\n"); > if (ret < 0) { > + g_free(block); > return ret; > } Alternative using g_autofree: -- >8 -- diff --git a/block/vdi.c b/block/vdi.c index 5627e7d764a..1cd8ae2ba99 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -612,7 +612,7 @@ vdi_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes, uint64_t data_offset; uint32_t bmap_first = VDI_UNALLOCATED; uint32_t bmap_last = VDI_UNALLOCATED; - uint8_t *block = NULL; + g_autofree uint8_t *block = NULL; uint64_t bytes_done = 0; int ret = 0; @@ -705,9 +705,6 @@ nonallocating_write: *header = s->header; vdi_header_to_le(header); ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader)); - g_free(block); - block = NULL; - if (ret < 0) { return ret; } ---
On Tuesday, 2021-03-09 at 12:09:55 +01, Philippe Mathieu-Daudé wrote: > On 3/9/21 11:21 AM, David Edmondson wrote: >> If a new bitmap entry is allocated, requiring the entire block to be >> written, avoiding leaking the buffer allocated for the block should >> the write fail. >> >> Signed-off-by: David Edmondson <david.edmondson@oracle.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Thanks. >> --- >> block/vdi.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/block/vdi.c b/block/vdi.c >> index 5627e7d764..2a6dc26124 100644 >> --- a/block/vdi.c >> +++ b/block/vdi.c >> @@ -690,6 +690,7 @@ nonallocating_write: >> >> logout("finished data write\n"); >> if (ret < 0) { >> + g_free(block); >> return ret; >> } > > Alternative using g_autofree: Newfangled witchy magic! I'm happy to change it if you think it beneficial. > -- >8 -- > diff --git a/block/vdi.c b/block/vdi.c > index 5627e7d764a..1cd8ae2ba99 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -612,7 +612,7 @@ vdi_co_pwritev(BlockDriverState *bs, uint64_t > offset, uint64_t bytes, > uint64_t data_offset; > uint32_t bmap_first = VDI_UNALLOCATED; > uint32_t bmap_last = VDI_UNALLOCATED; > - uint8_t *block = NULL; > + g_autofree uint8_t *block = NULL; > uint64_t bytes_done = 0; > int ret = 0; > > @@ -705,9 +705,6 @@ nonallocating_write: > *header = s->header; > vdi_header_to_le(header); > ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader)); > - g_free(block); > - block = NULL; > - > if (ret < 0) { > return ret; > } > --- dme.
On 3/9/21 12:58 PM, David Edmondson wrote: > On Tuesday, 2021-03-09 at 12:09:55 +01, Philippe Mathieu-Daudé wrote: > >> On 3/9/21 11:21 AM, David Edmondson wrote: >>> If a new bitmap entry is allocated, requiring the entire block to be >>> written, avoiding leaking the buffer allocated for the block should >>> the write fail. >>> >>> Signed-off-by: David Edmondson <david.edmondson@oracle.com> >> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Thanks. > >>> --- >>> block/vdi.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/block/vdi.c b/block/vdi.c >>> index 5627e7d764..2a6dc26124 100644 >>> --- a/block/vdi.c >>> +++ b/block/vdi.c >>> @@ -690,6 +690,7 @@ nonallocating_write: >>> >>> logout("finished data write\n"); >>> if (ret < 0) { >>> + g_free(block); >>> return ret; >>> } >> >> Alternative using g_autofree: > > Newfangled witchy magic! > > I'm happy to change it if you think it beneficial. I then saw the next patch which keeps modifying the same function, so this might not be a great improvement after all.
On 09/03/21 13:06, Philippe Mathieu-Daudé wrote: >> Newfangled witchy magic! >> >> I'm happy to change it if you think it beneficial. > > I then saw the next patch which keeps modifying the same > function, so this might not be a great improvement after > all. Yeah I was also going to suggest it but considering patch 2 it doesn't really flow well. paolo
diff --git a/block/vdi.c b/block/vdi.c index 5627e7d764..2a6dc26124 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -690,6 +690,7 @@ nonallocating_write: logout("finished data write\n"); if (ret < 0) { + g_free(block); return ret; }
If a new bitmap entry is allocated, requiring the entire block to be written, avoiding leaking the buffer allocated for the block should the write fail. Signed-off-by: David Edmondson <david.edmondson@oracle.com> --- block/vdi.c | 1 + 1 file changed, 1 insertion(+)