| Message ID | 20230727150705.2664464-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block/iscsi: Document why we use raw malloc() | expand |
On Thu, 27 Jul 2023 at 16:07, Peter Maydell <peter.maydell@linaro.org> wrote: > > In block/iscsi.c we use a raw malloc() call, which is unusual > given the project standard is to use the glib memory allocation > functions. Document why we do so, to avoid it being converted > to g_malloc() by mistake. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > There aren't many uses of raw malloc() in the codebase > other than third-party library sourcecode. Mostly we have > a comment noting when we're doing it deliberately. > --- > block/iscsi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/iscsi.c b/block/iscsi.c > index 34f97ab6460..5640c8b5657 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c > @@ -1058,6 +1058,7 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, > return NULL; > } > > + /* Must use malloc(): this is freed via scsi_free_scsi_task() */ > acb->task = malloc(sizeof(struct scsi_task)); > if (acb->task == NULL) { > error_report("iSCSI: Failed to allocate task for scsi command. %s", The other option here would be to use scsi_create_task(), if we're OK with requiring libiscsi 1.13 or better (that's 9 years old at this point, so should be OK...) thanks -- PMM
Am 27.07.2023 um 17:15 hat Peter Maydell geschrieben: > On Thu, 27 Jul 2023 at 16:07, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > In block/iscsi.c we use a raw malloc() call, which is unusual > > given the project standard is to use the glib memory allocation > > functions. Document why we do so, to avoid it being converted > > to g_malloc() by mistake. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > There aren't many uses of raw malloc() in the codebase > > other than third-party library sourcecode. Mostly we have > > a comment noting when we're doing it deliberately. Thanks, applied to the block branch. > The other option here would be to use scsi_create_task(), > if we're OK with requiring libiscsi 1.13 or better (that's > 9 years old at this point, so should be OK...) I wouldn't mind either way, but since this patch exists and the other doesn't, I'm applying this one for now. Kevin
On Fri, 18 Aug 2023 at 16:56, Kevin Wolf <kwolf@redhat.com> wrote: > > Am 27.07.2023 um 17:15 hat Peter Maydell geschrieben: > > On Thu, 27 Jul 2023 at 16:07, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > > > In block/iscsi.c we use a raw malloc() call, which is unusual > > > given the project standard is to use the glib memory allocation > > > functions. Document why we do so, to avoid it being converted > > > to g_malloc() by mistake. > > > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > > --- > > > There aren't many uses of raw malloc() in the codebase > > > other than third-party library sourcecode. Mostly we have > > > a comment noting when we're doing it deliberately. > > Thanks, applied to the block branch. > > > The other option here would be to use scsi_create_task(), > > if we're OK with requiring libiscsi 1.13 or better (that's > > 9 years old at this point, so should be OK...) > > I wouldn't mind either way, but since this patch exists and the other > doesn't, I'm applying this one for now. Thanks. I think scsi_create_task() would be cleaner, but it's just over the amount of change that I'd want to do without a test setup to exercise this code, which is why I stuck to the comment for now. -- PMM
diff --git a/block/iscsi.c b/block/iscsi.c index 34f97ab6460..5640c8b5657 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1058,6 +1058,7 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs, return NULL; } + /* Must use malloc(): this is freed via scsi_free_scsi_task() */ acb->task = malloc(sizeof(struct scsi_task)); if (acb->task == NULL) { error_report("iSCSI: Failed to allocate task for scsi command. %s",
In block/iscsi.c we use a raw malloc() call, which is unusual given the project standard is to use the glib memory allocation functions. Document why we do so, to avoid it being converted to g_malloc() by mistake. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- There aren't many uses of raw malloc() in the codebase other than third-party library sourcecode. Mostly we have a comment noting when we're doing it deliberately. --- block/iscsi.c | 1 + 1 file changed, 1 insertion(+)