diff mbox

[U-Boot,v2,1/5] sf: ops: Squash the malloc+memset combo

Message ID ab32ed8f-2813-45ff-b014-fdfd2107d624@CO9EHSMHS025.ehs.local
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Jagannadha Sutradharudu Teki Jan. 20, 2014, 2:29 p.m. UTC
Squash the malloc()+memset() combo in favor of calloc().

Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
Cc: Marek Vasut <marex@denx.de>
---
 drivers/mtd/spi/sf_ops.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Fabio Estevam Jan. 20, 2014, 2:35 p.m. UTC | #1
On Mon, Jan 20, 2014 at 12:29 PM, Jagannadha Sutradharudu Teki
<jagannadha.sutradharudu-teki@xilinx.com> wrote:
> Squash the malloc()+memset() combo in favor of calloc().
>
> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
>  drivers/mtd/spi/sf_ops.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 1f1bb36..abdb0ef 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -381,8 +381,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>         }
>
>         cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
> -       cmd = malloc(cmdsz);
> -       memset(cmd, 0, cmdsz);
> +       cmd = calloc(1, cmdsz);
> +       if (!cmd) {
> +               debug("SF: Failed to allocate cmd\n");
> +               return ret;

Shouldn't you return -ENOMEM instead?

Regards,

Fabio Estevam
Jagan Teki Jan. 20, 2014, 2:40 p.m. UTC | #2
On Mon, Jan 20, 2014 at 8:05 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Jan 20, 2014 at 12:29 PM, Jagannadha Sutradharudu Teki
> <jagannadha.sutradharudu-teki@xilinx.com> wrote:
>> Squash the malloc()+memset() combo in favor of calloc().
>>
>> Signed-off-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> Cc: Marek Vasut <marex@denx.de>
>> ---
>>  drivers/mtd/spi/sf_ops.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> index 1f1bb36..abdb0ef 100644
>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -381,8 +381,11 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>>         }
>>
>>         cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
>> -       cmd = malloc(cmdsz);
>> -       memset(cmd, 0, cmdsz);
>> +       cmd = calloc(1, cmdsz);
>> +       if (!cmd) {
>> +               debug("SF: Failed to allocate cmd\n");
>> +               return ret;
>
> Shouldn't you return -ENOMEM instead?

Yes - we can but anyway ret is -1 by default.
and sf code doesn't use -ve macros' as of now.
Fabio Estevam Jan. 20, 2014, 2:47 p.m. UTC | #3
On Mon, Jan 20, 2014 at 12:40 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> Shouldn't you return -ENOMEM instead?
>
> Yes - we can but anyway ret is -1 by default.
> and sf code doesn't use -ve macros' as of now.

-1 is not an propriate return error value in this case.

Regards,

Fabio Estevam
Jagan Teki Jan. 20, 2014, 3:03 p.m. UTC | #4
On Mon, Jan 20, 2014 at 8:17 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Mon, Jan 20, 2014 at 12:40 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> Shouldn't you return -ENOMEM instead?
>>
>> Yes - we can but anyway ret is -1 by default.
>> and sf code doesn't use -ve macros' as of now.
>
> -1 is not an propriate return error value in this case.

Yes it should be -11 - then we need to use -ENOMEM
diff mbox

Patch

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index 1f1bb36..abdb0ef 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -381,8 +381,11 @@  int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 	}
 
 	cmdsz = SPI_FLASH_CMD_LEN + flash->dummy_byte;
-	cmd = malloc(cmdsz);
-	memset(cmd, 0, cmdsz);
+	cmd = calloc(1, cmdsz);
+	if (!cmd) {
+		debug("SF: Failed to allocate cmd\n");
+		return ret;
+	}
 
 	cmd[0] = flash->read_cmd;
 	while (len) {