On 02/07/2024 11:00, lijiang wrote:
 On Thu, Jun 27, 2024 at 4:31 PM
<devel-request(a)lists.crash-utility.osci.io
<mailto:devel-request@lists.crash-utility.osci.io>> wrote:
 
     Date: Thu, 27 Jun 2024 11:18:29 +0800
     From: Li Zhijian <lizhijian(a)fujitsu.com <mailto:lizhijian@fujitsu.com>>
     Subject: [Crash-utility] [PATCH] list: fatal if -r isn't used in
              conjunction with -H or -h
     To: devel(a)lists.crash-utility.osci.io
<mailto:devel@lists.crash-utility.osci.io>
     Cc: Li Zhijian <lizhijian(a)fujitsu.com <mailto:lizhijian@fujitsu.com>>
     Message-ID: <20240627031829.235256-1-lizhijian(a)fujitsu.com
<mailto:20240627031829.235256-1-lizhijian@fujitsu.com>>
 
     Per the code, -r(LIST_HEAD_REVERSE) only work with LIST_HEAD_FORMAT
     which is set by -H or -h.
 
     Previously, if LIST_HEAD_FORMAT was not set, `list -r` will traverse the
     list in order, that doesn't obey the -r(reverse) semantics.
 
 According to the above descriptions, I guess that it should be:
 
 diff --git a/tools.c b/tools.c
 index 0f2db108838a..1022d579071a 100644
 --- a/tools.c
 +++ b/tools.c
 @@ -3370,6 +3370,7 @@ cmd_list(void)
                          break;
 
                  case 'r':
 +                       ld->flags |= LIST_HEAD_FORMAT;
 
Cool, It sounds more better/accurate.
"            -r For a list linked with list_head structures, traverse the
list",
"               in the reverse order by using the \"prev\" pointer
instead",
"               of \"next\".",
-r usage also says it's used for list_head structure. So implicitly enable
LIST_HEAD_REVERSE
is reasonable.
Thanks
Zhijian
  
>                           ld->flags |= LIST_HEAD_REVERSE;
>                          break;
> 
>   Also let's see the following code:
> 
>          if (ld->flags & LIST_HEAD_FORMAT) {
>                  ld->list_head_offset = ld->member_offset;
>                  if (ld->flags & LIST_HEAD_REVERSE)
>                          ld->member_offset = sizeof(void *);
>                  else
>                          ld->member_offset = 0;
> 
> Furthermore, the help page is:
> SYNOPSIS
>    list [[-o] offset][-e end][-[s|S] struct[.member[,member] [-l offset]] -[x|d]]
>         [-r|-B] [-h [-O head_offset]|-H] start
> 
> The '-r' looks like an optional option, just like '-h' and
'-H'.
> 
> Can you help double check?
> 
> Thanks
> Lianbo
> 
>     Add a further check to ensure -r is used in conjunction with -H or -h.
> 
>     Signed-off-by: Li Zhijian <lizhijian(a)fujitsu.com
<mailto:lizhijian@fujitsu.com>>
>     ---
>       help.c  | 3 ++-
>       tools.c | 3 +++
>       2 files changed, 5 insertions(+), 1 deletion(-)
> 
>     diff --git a/help.c b/help.c
>     index d80e843703c1..6f7e093cbef1 100644
>     --- a/help.c
>     +++ b/help.c
>     @@ -5977,7 +5977,8 @@ char *help__list[] = {
>       " ",
>       "           -x  Override the default output format with hexadecimal
format.",
>       "           -d  Override the default output format with decimal
format.",
>     -"           -r  For a list linked with list_head structures, traverse the
list",
>     +"           -r  Must be used in conjunction with either -H or -h.",
>     +"               For a list linked with list_head structures, traverse the
list",
>       "               in the reverse order by using the \"prev\"
pointer instead",
>       "               of \"next\".",
>       "           -B  Use the algorithm from R. P. Brent to detect loops instead
of",
>     diff --git a/tools.c b/tools.c
>     index 0f2db108838a..67977605c276 100644
>     --- a/tools.c
>     +++ b/tools.c
>     @@ -3451,6 +3451,9 @@ cmd_list(void)
>                      }
>              }
> 
>     +       if (ld->flags & LIST_HEAD_REVERSE && !(ld->flags &
LIST_HEAD_FORMAT))
>     +               error(FATAL, "-r must be used in conjunction with -H or
-h\n");
>     +
>              if (argerrs)
>                      cmd_usage(pc->curcmd, SYNOPSIS);
> 
>     -- 
>     2.29.2
>