Re: [Crash-utility] [PATCH v1] diskdump: add hook for additional checks on prstatus notes validity

On Tue, Sep 26, 2023 at 2:25 PM Aditya Gupta <adityag(a)linux.ibm.com&gt; 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?

Thanks. Lianbo > > > >