| Message ID | 20200513142359.147589-2-sjg@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: cbfs: Various clean-ups to CBFS implementation | expand |
Hi Simon, On Wed, May 13, 2020 at 10:24 PM Simon Glass <sjg at chromium.org> wrote: > > At present the result variable in the cbfs_priv is called 'result' as is > the local variable in a few functions. Change the latter to 'ret' which is > more common in U-Boot and avoids confusion. > > Signed-off-by: Simon Glass <sjg at chromium.org> > --- > > fs/cbfs/cbfs.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > index 1aa6f8ee84..70440aa80b 100644 > --- a/fs/cbfs/cbfs.c > +++ b/fs/cbfs/cbfs.c > @@ -145,18 +145,18 @@ static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, > priv->file_cache = NULL; > > while (size >= align) { > - int result; > + int ret; > u32 used; > > new_node = (struct cbfs_cachenode *) > malloc(sizeof(struct cbfs_cachenode)); > - result = file_cbfs_next_file(priv, start, size, align, new_node, > - &used); > + ret = file_cbfs_next_file(priv, start, size, align, new_node, > + &used); Could you also fix the return value description in the comment block of file_cbfs_next_file()? Now it only mentions 0 and 1, but it returns -1 when something goes wrong. > > - if (result < 0) { > + if (ret < 0) { > free(new_node); > return; > - } else if (result == 0) { > + } else if (ret == 0) { > free(new_node); > break; > } > @@ -341,15 +341,15 @@ const struct cbfs_cachenode *file_cbfs_find_uncached(uintptr_t end_of_rom, > align = priv->header.align; > > while (size >= align) { > - int result; > + int ret; > u32 used; > > - result = file_cbfs_next_file(priv, start, size, align, &node, > - &used); > + ret = file_cbfs_next_file(priv, start, size, align, &node, > + &used); > > - if (result < 0) > + if (ret < 0) > return NULL; > - else if (result == 0) > + else if (ret == 0) > break; > > if (!strcmp(name, node.name)) > -- Reviewed-by: Bin Meng <bmeng.cn at gmail.com> Regards, Bin
Hi Bin, On Tue, 19 May 2020 at 07:53, Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Simon, > > On Wed, May 13, 2020 at 10:24 PM Simon Glass <sjg at chromium.org> wrote: > > > > At present the result variable in the cbfs_priv is called 'result' as is > > the local variable in a few functions. Change the latter to 'ret' which is > > more common in U-Boot and avoids confusion. > > > > Signed-off-by: Simon Glass <sjg at chromium.org> > > --- > > > > fs/cbfs/cbfs.c | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c > > index 1aa6f8ee84..70440aa80b 100644 > > --- a/fs/cbfs/cbfs.c > > +++ b/fs/cbfs/cbfs.c > > @@ -145,18 +145,18 @@ static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, > > priv->file_cache = NULL; > > > > while (size >= align) { > > - int result; > > + int ret; > > u32 used; > > > > new_node = (struct cbfs_cachenode *) > > malloc(sizeof(struct cbfs_cachenode)); > > - result = file_cbfs_next_file(priv, start, size, align, new_node, > > - &used); > > + ret = file_cbfs_next_file(priv, start, size, align, new_node, > > + &used); > > Could you also fix the return value description in the comment block > of file_cbfs_next_file()? Now it only mentions 0 and 1, but it returns > -1 when something goes wrong. A later fixes the return values and the comments, as you may have seen. [..] Regards, Simon
diff --git a/fs/cbfs/cbfs.c b/fs/cbfs/cbfs.c index 1aa6f8ee84..70440aa80b 100644 --- a/fs/cbfs/cbfs.c +++ b/fs/cbfs/cbfs.c @@ -145,18 +145,18 @@ static void file_cbfs_fill_cache(struct cbfs_priv *priv, u8 *start, u32 size, priv->file_cache = NULL; while (size >= align) { - int result; + int ret; u32 used; new_node = (struct cbfs_cachenode *) malloc(sizeof(struct cbfs_cachenode)); - result = file_cbfs_next_file(priv, start, size, align, new_node, - &used); + ret = file_cbfs_next_file(priv, start, size, align, new_node, + &used); - if (result < 0) { + if (ret < 0) { free(new_node); return; - } else if (result == 0) { + } else if (ret == 0) { free(new_node); break; } @@ -341,15 +341,15 @@ const struct cbfs_cachenode *file_cbfs_find_uncached(uintptr_t end_of_rom, align = priv->header.align; while (size >= align) { - int result; + int ret; u32 used; - result = file_cbfs_next_file(priv, start, size, align, &node, - &used); + ret = file_cbfs_next_file(priv, start, size, align, &node, + &used); - if (result < 0) + if (ret < 0) return NULL; - else if (result == 0) + else if (ret == 0) break; if (!strcmp(name, node.name))
At present the result variable in the cbfs_priv is called 'result' as is the local variable in a few functions. Change the latter to 'ret' which is more common in U-Boot and avoids confusion. Signed-off-by: Simon Glass <sjg at chromium.org> --- fs/cbfs/cbfs.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)