Re: sdmmc: fix malloc error handling in sdmmc_mem_send_scr()

[Tobias Heider]

On Mon, Jan 10, 2022 at 04:20:36PM +0100, Stefan Sperling wrote:
> On Mon, Jan 10, 2022 at 03:50:45PM +0100, Tobias Heider wrote:
> > Makes sense. I also fixed the one in sdmmc_mem_send_cxd_data().
> 
> Doesn't build here, there a few errors like this:
> 
> /usr/src/sys/dev/sdmmc/sdmmc_mem.c:483:1: error: unused label 'out' 
> [-Werror,-Wu
> nused-label] 
> 
> I like Visa's idea of using early 'return ENOMEM' instead of goto.

I removed the unused goto labels and cleaned up the error handling.
We don't need to check for (ptr != NULL) and in one case we can
merge two 'if (error == 0)' blocks.

ok?

Index: sdmmc_mem.c
===================================================================
RCS file: /mount/openbsd/cvs/src/sys/dev/sdmmc/sdmmc_mem.c,v
retrieving revision 1.36
diff -u -p -r1.36 sdmmc_mem.c
--- sdmmc_mem.c 27 Mar 2021 14:36:28 -0000      1.36
+++ sdmmc_mem.c 10 Jan 2022 05:20:37 -0000
@@ -466,7 +466,7 @@ sdmmc_mem_send_scr(struct sdmmc_softc *s
 
        ptr = malloc(datalen, M_DEVBUF, M_NOWAIT | M_ZERO);
        if (ptr == NULL)
-               goto out;
+               return ENOMEM;
 
        memset(&cmd, 0, sizeof(cmd));
        cmd.c_data = ptr;
@@ -480,9 +480,7 @@ sdmmc_mem_send_scr(struct sdmmc_softc *s
        if (error == 0)
                memcpy(scr, ptr, datalen);
 
-out:
-       if (ptr != NULL)
-               free(ptr, M_DEVBUF, datalen);
+       free(ptr, M_DEVBUF, datalen);
 
        return error;
 }
@@ -528,10 +526,8 @@ sdmmc_mem_send_cxd_data(struct sdmmc_sof
        int error = 0;
 
        ptr = malloc(datalen, M_DEVBUF, M_NOWAIT | M_ZERO);
-       if (ptr == NULL) {
-               error = ENOMEM;
-               goto out;
-       }
+       if (ptr == NULL)
+               return ENOMEM;
 
        memset(&cmd, 0, sizeof(cmd));
        cmd.c_data = ptr;
@@ -549,9 +545,7 @@ sdmmc_mem_send_cxd_data(struct sdmmc_sof
        if (error == 0)
                memcpy(data, ptr, datalen);
 
-out:
-       if (ptr != NULL)
-               free(ptr, M_DEVBUF, datalen);
+       free(ptr, M_DEVBUF, datalen);
 
        return error;
 }
@@ -608,7 +602,7 @@ sdmmc_mem_sd_switch(struct sdmmc_functio
 
        ptr = malloc(statlen, M_DEVBUF, M_NOWAIT | M_ZERO);
        if (ptr == NULL)
-               goto out;
+               return ENOMEM;
 
        memset(&cmd, 0, sizeof(cmd));
        cmd.c_data = ptr;
@@ -620,15 +614,12 @@ sdmmc_mem_sd_switch(struct sdmmc_functio
        cmd.c_flags = SCF_CMD_ADTC | SCF_CMD_READ | SCF_RSP_R1;
 
        error = sdmmc_mmc_command(sc, &cmd);
-       if (error == 0)
+       if (error == 0) {
                memcpy(status, ptr, statlen);
-
-out:
-       if (ptr != NULL)
-               free(ptr, M_DEVBUF, statlen);
-
-       if (error == 0)
                sdmmc_be512_to_bitfield512(status);
+       }
+
+       free(ptr, M_DEVBUF, statlen);
 
        return error;
 }