Re: [PATCH v1 0/3] Add valgrind support for the crash's custom memory

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

 



Hi Lianbo, Hatayama-san,

-----Original Message-----
> >> For the 'make lzo', the cflag '-DVALGRIND' is also added here after the step1, is that expected?
> > As written in the README, these targets are sticky, so it's expected:
> >
> >   All of the alternate build commands above are "sticky" in that the
> >   special "make" targets only have to be entered one time; all subsequent
> >   builds will follow suit.
> >
> > AFAIK, the only command that can drop a target is "make nowarn", otherwise
> > we can drop "lzo" and so on by removing CFLAGS.extra and LDFLAGS.extra for
> > the present.
> >
> Seems yes. Is it possible to separate these CFLAGS? And we may put them together when
> it is needed, For example:
> 
> make lzo           (-DLZO)
> make valgrind      (-DVALGRIND)
> make lzo_valgrind  (-DVALGRIND -DLZO)

sorry I'm not sure what you mean with the "separate these CFLAGS", but
"make lzo valgrind" builds a crash with -DLZO first, and then rebuilds
it with -DLZO and -DVALGRIND.  Doesn't this satisfy your expectation?


However, I found that the "make lzo valgrind" might not work well with
this v1 patch, thanks to Lianbo.

Hatayama-san, do we need to unlink tools.o like lzo and snappy below
to rebuild tools.c with -DVALGRIND for e.g. "make lzo valgrind" ?

1757         if ((lzo || snappy) &&
1758             file_exists("diskdump.o") && (unlink("diskdump.o") < 0)) {
1759                 perror("diskdump.o");
1760                 return;
1761         }

> 
> But I'm not sure if it looks more reasonable. Anyway, this is another issue.

Yes, if you need to change the current target handling, let's discuss it
separately from this patchset.

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