On Tue, Sep 26, 2023 at 03:51:31PM +0800, lijiang wrote:
> On Tue, Sep 26, 2023 at 2:25 PM Aditya Gupta <adityag@xxxxxxxxxxxxx> wrote:
>
> > >
> > > Got a warning as below:
> > >
> > > cc -c -g -DX86_64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
> > > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> > > -Wformat-security
> > > diskdump.c:145:5: warning: no previous prototype for
> > > ‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
> > > 145 | int diskdump_is_cpu_prstatus_valid(int cpu)
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> >
> > Will fix in V2. Will add a declaration in defs.h, just above
> > 'have_crash_notes'
> > declaration, or should I add the declaration at the end of all diskdump.c
> > declarations in defs.h ?
> >
> >
> For now they are only invoked in the local file, not used in the other
> modules, so it could be good to add a 'static' keyword and declare them in
> the local file. For example:
>
> warning[1]:
>
> cc -c -g -DPPC64 -m64 -DLZO -DGDB_10_2 ppc64.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> -Wformat-security
> ppc64.c:305:5: warning: no previous prototype for
> ‘ppc64_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
> 305 | int ppc64_is_cpu_prstatus_valid(int cpu)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> static int is_opal_context(ulong sp, ulong nip);
> +static int ppc64_is_cpu_prstatus_valid(int cpu);
> void opalmsg(void);
>
> static int is_opal_context(ulong sp, ulong nip)
> @@ -298,6 +299,15 @@ struct machine_specific book3e_machine_specific = {
> .is_vmaddr = book3e_is_vmaddr,
> };
>
> +/**
> + * No additional checks are required on PPC64, for checking if PRSTATUS
> notes
> + * is valid
> + */
> +static int ppc64_is_cpu_prstatus_valid(int cpu)
> +{
> + return TRUE;
> +}
>
>
> warning[2]:
>
> cc -c -g -DPPC64 -m64 -DLZO -DGDB_10_2 diskdump.c -Wall -O2
> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> -Wformat-security
> diskdump.c:145:5: warning: no previous prototype for
> ‘diskdump_is_cpu_prstatus_valid’ [-Wmissing-prototypes]
> 145 | int diskdump_is_cpu_prstatus_valid(int cpu)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> static int valid_note_address(unsigned char *);
> +static int diskdump_is_cpu_prstatus_valid(int cpu);
>
> /* For split dumpfile */
> static struct diskdump_data **dd_list = NULL;
> @@ -142,13 +143,22 @@ int have_crash_notes(int cpu)
> return TRUE;
> }
>
> +static int diskdump_is_cpu_prstatus_valid(int cpu)
> +{
> + static int crash_notes_exists = -1;
> +
> ...
> }
>
> What do you think about it?
Yes, makes sense. Thanks. Will add static declarations for them.
Will send a V2 after testing the changes, also I noticed a possible bug in
netdump.c (line 104) due to same 'have_crash_notes' check, may have to rename
'diskdump_is_cpu_prstatus_valid' to something like
'default_is_cpu_prstatus_valid' or something similar, if it is going to be used
in both places.
Thanks,
- Aditya
>
> Thanks.
> Lianbo
>
> >
> > >
> >
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
