Re: [PATCH v3 09/18] pxe: Tidy up code style a little in pxe_utils

2021-11-12 Thread Tom Rini
On Thu, Oct 14, 2021 at 12:48:02PM -0600, Simon Glass wrote:

> There are a few more blank lines than makes sense for readability. Also
> free() handles a NULL pointer so drop the pointless checks.
> 
> Signed-off-by: Simon Glass 
> Reviewed-by: Artem Lapkin 
> Tested-by:  Artem Lapkin 
> Reviewed-by: Ramon Fried 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 09/18] pxe: Tidy up code style a little in pxe_utils

2021-11-09 Thread Ramon Fried
On Thu, Oct 14, 2021 at 9:50 PM Simon Glass  wrote:
>
> There are a few more blank lines than makes sense for readability. Also
> free() handles a NULL pointer so drop the pointless checks.
>
> Signed-off-by: Simon Glass 
> ---
>
> (no changes since v1)
>
>  boot/pxe_utils.c | 66 ++--
>  1 file changed, 13 insertions(+), 53 deletions(-)
>
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index 7a2213a5925..9f3edeab06a 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -51,7 +51,6 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
>
> if (outbuf_len < 21) {
> printf("outbuf is too small (%zd < 21)\n", outbuf_len);
> -
> return -ENOSPC;
> }
>
> @@ -91,12 +90,10 @@ static int get_bootfile_path(const char *file_path, char 
> *bootfile_path,
> goto ret;
>
> bootfile = from_env("bootfile");
> -
> if (!bootfile)
> goto ret;
>
> last_slash = strrchr(bootfile, '/');
> -
> if (!last_slash)
> goto ret;
>
> @@ -140,7 +137,6 @@ static int get_relfile(struct pxe_context *ctx, const 
> char *file_path,
>
> err = get_bootfile_path(file_path, relfile, sizeof(relfile),
> ctx->allow_abs_path);
> -
> if (err < 0)
> return err;
>
> @@ -181,7 +177,6 @@ int get_pxe_file(struct pxe_context *ctx, const char 
> *file_path,
> char *buf;
>
> err = get_relfile(ctx, file_path, file_addr);
> -
> if (err < 0)
> return err;
>
> @@ -190,7 +185,6 @@ int get_pxe_file(struct pxe_context *ctx, const char 
> *file_path,
>  * and add the NUL byte.
>  */
> tftp_filesize = from_env("filesize");
> -
> if (!tftp_filesize)
> return -ENOENT;
>
> @@ -253,7 +247,6 @@ static int get_relfile_envaddr(struct pxe_context *ctx, 
> const char *file_path,
> char *envaddr;
>
> envaddr = from_env(envaddr_name);
> -
> if (!envaddr)
> return -ENOENT;
>
> @@ -276,7 +269,6 @@ static struct pxe_label *label_create(void)
> struct pxe_label *label;
>
> label = malloc(sizeof(struct pxe_label));
> -
> if (!label)
> return NULL;
>
> @@ -300,30 +292,14 @@ static struct pxe_label *label_create(void)
>   */
>  static void label_destroy(struct pxe_label *label)
>  {
> -   if (label->name)
> -   free(label->name);
> -
> -   if (label->kernel)
> -   free(label->kernel);
> -
> -   if (label->config)
> -   free(label->config);
> -
> -   if (label->append)
> -   free(label->append);
> -
> -   if (label->initrd)
> -   free(label->initrd);
> -
> -   if (label->fdt)
> -   free(label->fdt);
> -
> -   if (label->fdtdir)
> -   free(label->fdtdir);
> -
> -   if (label->fdtoverlays)
> -   free(label->fdtoverlays);
> -
> +   free(label->name);
> +   free(label->kernel);
> +   free(label->config);
> +   free(label->append);
> +   free(label->initrd);
> +   free(label->fdt);
> +   free(label->fdtdir);
> +   free(label->fdtoverlays);
> free(label);
>  }
>
> @@ -359,7 +335,6 @@ static int label_localboot(struct pxe_label *label)
> char *localcmd;
>
> localcmd = from_env("localcmd");
> -
> if (!localcmd)
> return -ENOENT;
>
> @@ -718,8 +693,8 @@ static int label_boot(struct pxe_context *ctx, struct 
> pxe_label *label)
> unmap_sysmem(buf);
>
>  cleanup:
> -   if (fit_addr)
> -   free(fit_addr);
> +   free(fit_addr);
> +
> return 1;
>  }
>
> @@ -832,7 +807,6 @@ static char *get_string(char **p, struct token *t, char 
> delim, int lower)
>  */
> b = *p;
> e = *p;
> -
> while (*e) {
> if ((delim == ' ' && isspace(*e)) || delim == *e)
> break;
> @@ -858,11 +832,8 @@ static char *get_string(char **p, struct token *t, char 
> delim, int lower)
>
> t->val[len] = '\0';
>
> -   /*
> -* Update *p so the caller knows where to continue scanning.
> -*/
> +   /* Update *p so the caller knows where to continue scanning */
> *p = e;
> -
> t->type = T_STRING;
>
> return t->val;
> @@ -988,7 +959,6 @@ static int parse_integer(char **c, int *dst)
> char *s = *c;
>
> get_token(c, , L_SLITERAL);
> -
> if (t.type != T_STRING) {
> printf("Expected string: %.*s\n", (int)(*c - s), s);
> return -EINVAL;
> @@ -1022,14 +992,12 @@ static int handle_include(struct pxe_context *ctx, 
> char **c, unsigned long base,
> int ret;
>
> err = parse_sliteral(c, _path);
> -
> if (err < 0) {
> printf("Expected include path: %.*s\n", (int)(*c - s), s);
>   

Re: [PATCH v3 09/18] pxe: Tidy up code style a little in pxe_utils

2021-10-18 Thread art

OK

Reviewed and Tested on -master Mon 18 Oct 2021 04:40:29 PM CST
Reviewed-by: Artem Lapkin 
Tested-by:  Artem Lapkin 




[PATCH v3 09/18] pxe: Tidy up code style a little in pxe_utils

2021-10-14 Thread Simon Glass
There are a few more blank lines than makes sense for readability. Also
free() handles a NULL pointer so drop the pointless checks.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 boot/pxe_utils.c | 66 ++--
 1 file changed, 13 insertions(+), 53 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 7a2213a5925..9f3edeab06a 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -51,7 +51,6 @@ int format_mac_pxe(char *outbuf, size_t outbuf_len)
 
if (outbuf_len < 21) {
printf("outbuf is too small (%zd < 21)\n", outbuf_len);
-
return -ENOSPC;
}
 
@@ -91,12 +90,10 @@ static int get_bootfile_path(const char *file_path, char 
*bootfile_path,
goto ret;
 
bootfile = from_env("bootfile");
-
if (!bootfile)
goto ret;
 
last_slash = strrchr(bootfile, '/');
-
if (!last_slash)
goto ret;
 
@@ -140,7 +137,6 @@ static int get_relfile(struct pxe_context *ctx, const char 
*file_path,
 
err = get_bootfile_path(file_path, relfile, sizeof(relfile),
ctx->allow_abs_path);
-
if (err < 0)
return err;
 
@@ -181,7 +177,6 @@ int get_pxe_file(struct pxe_context *ctx, const char 
*file_path,
char *buf;
 
err = get_relfile(ctx, file_path, file_addr);
-
if (err < 0)
return err;
 
@@ -190,7 +185,6 @@ int get_pxe_file(struct pxe_context *ctx, const char 
*file_path,
 * and add the NUL byte.
 */
tftp_filesize = from_env("filesize");
-
if (!tftp_filesize)
return -ENOENT;
 
@@ -253,7 +247,6 @@ static int get_relfile_envaddr(struct pxe_context *ctx, 
const char *file_path,
char *envaddr;
 
envaddr = from_env(envaddr_name);
-
if (!envaddr)
return -ENOENT;
 
@@ -276,7 +269,6 @@ static struct pxe_label *label_create(void)
struct pxe_label *label;
 
label = malloc(sizeof(struct pxe_label));
-
if (!label)
return NULL;
 
@@ -300,30 +292,14 @@ static struct pxe_label *label_create(void)
  */
 static void label_destroy(struct pxe_label *label)
 {
-   if (label->name)
-   free(label->name);
-
-   if (label->kernel)
-   free(label->kernel);
-
-   if (label->config)
-   free(label->config);
-
-   if (label->append)
-   free(label->append);
-
-   if (label->initrd)
-   free(label->initrd);
-
-   if (label->fdt)
-   free(label->fdt);
-
-   if (label->fdtdir)
-   free(label->fdtdir);
-
-   if (label->fdtoverlays)
-   free(label->fdtoverlays);
-
+   free(label->name);
+   free(label->kernel);
+   free(label->config);
+   free(label->append);
+   free(label->initrd);
+   free(label->fdt);
+   free(label->fdtdir);
+   free(label->fdtoverlays);
free(label);
 }
 
@@ -359,7 +335,6 @@ static int label_localboot(struct pxe_label *label)
char *localcmd;
 
localcmd = from_env("localcmd");
-
if (!localcmd)
return -ENOENT;
 
@@ -718,8 +693,8 @@ static int label_boot(struct pxe_context *ctx, struct 
pxe_label *label)
unmap_sysmem(buf);
 
 cleanup:
-   if (fit_addr)
-   free(fit_addr);
+   free(fit_addr);
+
return 1;
 }
 
@@ -832,7 +807,6 @@ static char *get_string(char **p, struct token *t, char 
delim, int lower)
 */
b = *p;
e = *p;
-
while (*e) {
if ((delim == ' ' && isspace(*e)) || delim == *e)
break;
@@ -858,11 +832,8 @@ static char *get_string(char **p, struct token *t, char 
delim, int lower)
 
t->val[len] = '\0';
 
-   /*
-* Update *p so the caller knows where to continue scanning.
-*/
+   /* Update *p so the caller knows where to continue scanning */
*p = e;
-
t->type = T_STRING;
 
return t->val;
@@ -988,7 +959,6 @@ static int parse_integer(char **c, int *dst)
char *s = *c;
 
get_token(c, , L_SLITERAL);
-
if (t.type != T_STRING) {
printf("Expected string: %.*s\n", (int)(*c - s), s);
return -EINVAL;
@@ -1022,14 +992,12 @@ static int handle_include(struct pxe_context *ctx, char 
**c, unsigned long base,
int ret;
 
err = parse_sliteral(c, _path);
-
if (err < 0) {
printf("Expected include path: %.*s\n", (int)(*c - s), s);
return err;
}
 
err = get_pxe_file(ctx, include_path, base);
-
if (err < 0) {
printf("Couldn't retrieve %s\n", include_path);
return err;
@@ -1079,7 +1047,6 @@ static int parse_menu(struct pxe_context *ctx, char **c, 
struct pxe_menu *cfg,
printf("Ignoring malformed menu command: