| Message ID | 20200504231732.98778-14-sjg@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Tidy up the 'bd' command. | expand |
Hi Simon, On Tue, May 5, 2020 at 7:19 AM Simon Glass <sjg at chromium.org> wrote: > > This function only has three lines of code in it so inline it. What's our guideline for inlining functions? To me if there is only one line, inline the function looks good to me. But 3 lines? I am not sure. > > Signed-off-by: Simon Glass <sjg at chromium.org> > --- > > cmd/bdinfo.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > Regards, Bin
On Wed, May 06, 2020 at 04:09:33PM +0800, Bin Meng wrote: > Hi Simon, > > On Tue, May 5, 2020 at 7:19 AM Simon Glass <sjg at chromium.org> wrote: > > > > This function only has three lines of code in it so inline it. > > What's our guideline for inlining functions? To me if there is only > one line, inline the function looks good to me. But 3 lines? I am not > sure. > > > > > Signed-off-by: Simon Glass <sjg at chromium.org> > > --- > > > > cmd/bdinfo.c | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) I'm ambivalent on if this should be moved in to the caller or not (I first read inlining as marking with inline). It kinda matters how the overall function looks when you're done.
Hi Bin, On Wed, 6 May 2020 at 02:09, Bin Meng <bmeng.cn at gmail.com> wrote: > > Hi Simon, > > On Tue, May 5, 2020 at 7:19 AM Simon Glass <sjg at chromium.org> wrote: > > > > This function only has three lines of code in it so inline it. > > What's our guideline for inlining functions? To me if there is only > one line, inline the function looks good to me. But 3 lines? I am not > sure. Me neither. But in this case I want to get all the code into once place and the end result is a function with about 30 lines, which seems OK. > > > > > Signed-off-by: Simon Glass <sjg at chromium.org> > > --- > > > > cmd/bdinfo.c | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > Regards, SImon
diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index b65eec332d..f22dc7a129 100644 --- a/cmd/bdinfo.c +++ b/cmd/bdinfo.c @@ -87,13 +87,6 @@ static void print_bi_dram(const bd_t *bd) #endif } -static void print_bi_flash(const bd_t *bd) -{ - print_num("flashstart", (ulong)bd->bi_flashstart); - print_num("flashsize", (ulong)bd->bi_flashsize); - print_num("flashoffset", (ulong)bd->bi_flashoffset); -} - static void print_eth_ip_addr(void) { #if defined(CONFIG_CMD_NET) @@ -135,7 +128,9 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) print_bi_dram(bd); print_num("memstart", (ulong)bd->bi_memstart); print_lnum("memsize", (u64)bd->bi_memsize); - print_bi_flash(bd); + print_num("flashstart", (ulong)bd->bi_flashstart); + print_num("flashsize", (ulong)bd->bi_flashsize); + print_num("flashoffset", (ulong)bd->bi_flashoffset); print_eth_ip_addr(); printf("baudrate = %u bps\n", gd->baudrate); print_num("relocaddr", gd->relocaddr);
This function only has three lines of code in it so inline it. Signed-off-by: Simon Glass <sjg at chromium.org> --- cmd/bdinfo.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)