On Wed, Jul 23, 2025 at 7:09 PM Aditya Gupta <adit...@linux.ibm.com> wrote:

> On 25/07/23 04:19PM, Aditya Gupta wrote:
> > Hi Lianbo,
> >
> > On 25/07/23 09:53AM, lijiang wrote:
> > > <...snip...>
> > >
> > > >
> > >  I have seen two versions of the implementation, another one is in
> > > netdump.c. Can we do it like this?
> > >
> > > bool is_vmcoreinfo_empty(void)
> > > {
> > >     if (dd && dd->sub_header_kdump)
> > >         return (dd->sub_header_kdump->size_vmcoreinfo == 0);
> > >     if (nd)
> > >         return (nd->size_vmcoreinfo == 0);
> > >     return true;
> > > }
> > >
> > > And implement it in a common file, E.g: kernel.c, put its definition to
> > > defs.h. So that we can call this one in netdump.c and diskdump.c.
> > > What do you think?
> >
> > Sure, makes sense. Will do it.
>
> On a second thought, i am re-thinking this since have to move 'dd' and
> 'nd' symbols also to defs.h, which currently are statics in diskdump.c
> and netdump.c
>
> The proposed v2 with only 1 is_vmcoreinfo_empty implementation:
>
> https://github.com/adi-g15-ibm/crash/commit/e579a8b0e79da409ff8f354bfba6104edbaa89f4
>
> What do you think Lianbo ? Is that okay ?
>
>
I tried your repo, and the build failed:

kernel.c: In function ‘is_vmcoreinfo_empty’:
kernel.c:11982:21: error: invalid use of undefined type ‘struct
diskdump_data’
11982 |         if (dd && dd->sub_header_kdump)
      |                     ^~
kernel.c:11983:27: error: invalid use of undefined type ‘struct
diskdump_data’
11983 |                 return (dd->sub_header_kdump->size_vmcoreinfo == 0);
      |                           ^~
kernel.c:11985:27: error: invalid use of undefined type ‘struct vmcore_data’
11985 |                 return (nd->size_vmcoreinfo == 0);
      |                           ^~
make[5]: *** [Makefile:421: kernel.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [Makefile:2237: gdb] Error 2
make[3]: *** [Makefile:11028: all-gdb] Error 2
make[2]: *** [Makefile:1034: all] Error 2

crash build failed

make[1]: *** [Makefile:316: gdb_merge] Error 1
make: *** [Makefile:307: all] Error 2

I'm afraid that needs more changes.

BTW: if there are significant code changes, let's try renaming
the is_vmcoreinfo_empty() to is_netdump/diskdump_vmcoreinfo_empty(), and
calling them separately in netdump.c/diskdump.c. Anyway, this depends on
your tests, let's see which one is simpler and more readable.

Thanks
Lianbo
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to