Re: [PATCH 1/5] zfs: Fix malloc() success check

[mwleeds]

On Sunday, April 7th, 2024 at 4:22 AM, Igor Opaniuk  
wrote:

> Hi Phaedrus,
> 
> On Sun, Apr 7, 2024 at 4:00 AM mwle...@mailtundra.com wrote:
> 
> > This code was hitting the error code path whenever malloc() succeeded
> > rather than when it failed, so presumably this part of the code hasn't
> > been tested. I had to apply this fix (and others) to get U-Boot to boot
> > from ZFS on an Nvidia Jetson TX2 NX SoM (an aarch64 computer).
> > 
> > Signed-off-by: Phaedrus Leeds mwle...@mailtundra.com
> > Tested-by: Phaedrus Leeds mwle...@mailtundra.com
> 
> It's an abuse of the Tested-by tag. If you are the author of the patch,
> that obviously implies that you tested it before sending to ML.
> Signed-off-by is enough in this case.
> 

That makes sense. Sorry I'm a bit new to this way of submitting patches and 
more accustomed to pull requests. It seems like a minor thing though; should I 
re-submit the patches?

> ---
> 
> > fs/zfs/zfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/zfs/zfs.c b/fs/zfs/zfs.c
> > index 1fec96cd5c..14779dee32 100644
> > --- a/fs/zfs/zfs.c
> > +++ b/fs/zfs/zfs.c
> > @@ -648,21 +648,21 @@ dmu_read(dnode_end_t *dn, uint64_t blkid, void **buf,
> > if (bp_array != dn->dn.dn_blkptr) {
> > free(bp_array);
> > bp_array = 0;
> > }
> > 
> > if (BP_IS_HOLE(bp)) {
> > size_t size = zfs_to_cpu16(dn->dn.dn_datablkszsec,
> > 
> > dn->endian)
> > << SPA_MINBLOCKSHIFT;
> > *buf = malloc(size);
> > - if (*buf) {
> > + if (!*buf) {
> > err = ZFS_ERR_OUT_OF_MEMORY;
> > break;
> > }
> > memset(*buf, 0, size);
> > endian = (zfs_to_cpu64(bp->blk_prop, endian) >>
> > 63) & 1;
> > break;
> > }
> > if (level == 0) {
> > err = zio_read(bp, endian, buf, 0, data);
> > endian = (zfs_to_cpu64(bp->blk_prop, endian) >>
> > 63) & 1;
> > --
> > 2.44.0
> 
> 
> --
> Best regards - Atentamente - Meilleures salutations
> 
> Igor Opaniuk
> 
> mailto: igor.opan...@gmail.com
> skype: igor.opanyuk
> https://www.linkedin.com/in/iopaniuk http://ua.linkedin.com/in/iopaniuk

Re: [PATCH 1/5] zfs: Fix malloc() success check

[Igor Opaniuk]

Hi Phaedrus,

On Sun, Apr 7, 2024 at 4:00 AM  wrote:

> This code was hitting the error code path whenever malloc() succeeded
> rather than when it failed, so presumably this part of the code hasn't
> been tested. I had to apply this fix (and others) to get U-Boot to boot
> from ZFS on an Nvidia Jetson TX2 NX SoM (an aarch64 computer).
>
> Signed-off-by: Phaedrus Leeds 
> Tested-by: Phaedrus Leeds 
>
It's an abuse of the Tested-by tag. If you are the author of the patch,
that obviously implies that you tested it before sending to ML.
Signed-off-by is enough in this case.

---
>  fs/zfs/zfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/zfs/zfs.c b/fs/zfs/zfs.c
> index 1fec96cd5c..14779dee32 100644
> --- a/fs/zfs/zfs.c
> +++ b/fs/zfs/zfs.c
> @@ -648,21 +648,21 @@ dmu_read(dnode_end_t *dn, uint64_t blkid, void **buf,
> if (bp_array != dn->dn.dn_blkptr) {
> free(bp_array);
> bp_array = 0;
> }
>
> if (BP_IS_HOLE(bp)) {
> size_t size = zfs_to_cpu16(dn->dn.dn_datablkszsec,
>
>   dn->endian)
> << SPA_MINBLOCKSHIFT;
> *buf = malloc(size);
> -   if (*buf) {
> +   if (!*buf) {
> err = ZFS_ERR_OUT_OF_MEMORY;
> break;
> }
> memset(*buf, 0, size);
> endian = (zfs_to_cpu64(bp->blk_prop, endian) >>
> 63) & 1;
> break;
> }
> if (level == 0) {
> err = zio_read(bp, endian, buf, 0, data);
> endian = (zfs_to_cpu64(bp->blk_prop, endian) >>
> 63) & 1;
> --
> 2.44.0
>
>

-- 
Best regards - Atentamente - Meilleures salutations

Igor Opaniuk

mailto: igor.opan...@gmail.com
skype: igor.opanyuk
https://www.linkedin.com/in/iopaniuk 

[PATCH 1/5] zfs: Fix malloc() success check

[mwleeds]

This code was hitting the error code path whenever malloc() succeeded
rather than when it failed, so presumably this part of the code hasn't
been tested. I had to apply this fix (and others) to get U-Boot to boot
from ZFS on an Nvidia Jetson TX2 NX SoM (an aarch64 computer).

Signed-off-by: Phaedrus Leeds 
Tested-by: Phaedrus Leeds 
---
 fs/zfs/zfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/zfs/zfs.c b/fs/zfs/zfs.c
index 1fec96cd5c..14779dee32 100644
--- a/fs/zfs/zfs.c
+++ b/fs/zfs/zfs.c
@@ -648,21 +648,21 @@ dmu_read(dnode_end_t *dn, uint64_t blkid, void **buf,
if (bp_array != dn->dn.dn_blkptr) {
free(bp_array);
bp_array = 0;
}
 
if (BP_IS_HOLE(bp)) {
size_t size = zfs_to_cpu16(dn->dn.dn_datablkszsec,

dn->endian)
<< SPA_MINBLOCKSHIFT;
*buf = malloc(size);
-   if (*buf) {
+   if (!*buf) {
err = ZFS_ERR_OUT_OF_MEMORY;
break;
}
memset(*buf, 0, size);
endian = (zfs_to_cpu64(bp->blk_prop, endian) >> 63) & 1;
break;
}
if (level == 0) {
err = zio_read(bp, endian, buf, 0, data);
endian = (zfs_to_cpu64(bp->blk_prop, endian) >> 63) & 1;
-- 
2.44.0