Re: [PATCH v2] Fix 'waitq' command for 4.13 and later kernels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----Original Message-----
> On Thu, Jun 24, 2021 at 12:51 PM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab@xxxxxxx> wrote:
> >
> > -----Original Message-----
> > > > >       } else {
> > > > > -             return;
> > > > > +             error(FATAL, "cannot determine wait queue structure\n");
> > > >
> > > > oh, I should have checked the replacement.. this emits compilation warnings:
> > > >
> > > > $ make clean ; make warn
> > > > ...
> > > > cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  kernel.c -Wall -O2 -Wstrict-prototypes
> -Wmissing-prototypes
> > > -fstack-protector -Wformat-security
> > > > kernel.c: In function 'cmd_waitq':
> > > > kernel.c:9380:6: warning: 'start_index' may be used uninitialized in this function
> > > [-Wmaybe-uninitialized]
> > > >   int start_index;  /* where to start in wq array */
> > > >       ^~~~~~~~~~~
> > > > kernel.c:9454:22: warning: 'task_offset' may be used uninitialized in this function
> > > [-Wmaybe-uninitialized]
> > > >    readmem(wq_list[i] + task_offset, KVADDR, &task,
> > > >            ~~~~~~~~~~~^~~~~~~~~~~~~
> > > > kernel.c:9378:8: note: 'task_offset' was declared here
> > > >   ulong task_offset;  /* offset of task in wq element */
> > > >         ^~~~~~~~~~~
> > > >
> > > > Hmm, in this case, I'd like to put the return back when merging.
> > > > Lianbo, is this ok?
> > > >
> > > >         } else {
> > > >                 error(FATAL, "cannot determine wait queue structure\n");
> > >
> > > After the log level is set to FATAL, it will exit, and the "return"
> > > has no chance to execute . So it could be good
> > > to use the log level "WARNING or INFO" for error()?
> >
> > I thought this is actually a fatal level error for the waitq command,
> > so we should use FATAL normally, and put return with the comment to
> > avoid confusion.  It's a little rough but lesser effort.
> >
> > +               return; /* just to suppress compilation warings */
> >
> > And the outputs are below respectively, I think "WARNING" in a message
> > implies that there is a problem but the command can continue somehow.
> > I don't want to use it in this case.
> >
> > crash> waitq kauditd_wait
> > waitq: cannot determine wait queue structure          // INFO
> > waitq: WARNING: cannot determine wait queue structure // WARNING
> > waitq: cannot determine wait queue structure          // FATAL
> >
> > So I would prefer FATAL.  INFO is better?
> 
> Maybe we should keep the original changes:
> -             return;
> +             error(FATAL, "cannot determine wait queue structure\n");
> 
> And fix the warnings directly in the kernel.c(dump_waitq()) as below:
> 
> -       ulong task_offset;              /* offset of task in wq element */
> +       ulong task_offset = 0;          /* offset of task in wq element */
>         int cnt;                        /* # elems on Queue */
> -       int start_index;                /* where to start in wq array */
> +       int start_index = -1;           /* where to start in wq array */

OK, I see.  Let's go with this reasonable fix.

Thanks,
Kazu


--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux