Messages in this thread Patch in this message | | | Date | Fri, 26 Jul 2019 14:43:10 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH v3 04/13] recordmcount: Rewrite error/success handling |
| |
On Fri, 26 Jul 2019 18:37:11 +0000 Matt Helsley <mhelsley@vmware.com> wrote: > >> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h > >> index c1e1b04b4871..909a3e4775c2 100644 > >> --- a/scripts/recordmcount.h > >> +++ b/scripts/recordmcount.h > >> @@ -24,7 +24,9 @@ > >> #undef mcount_adjust > >> #undef sift_rel_mcount > >> #undef nop_mcount > >> +#undef missing_sym > >> #undef find_secsym_ndx > >> +#undef already_has_rel_mcount > > > > Why do we need these as defines? Can't you just have a single: > > > > const char *already_has_mcount = "success"; > > > > in recordmcount.c before recordmcount.h is included? > > > > And same for missing_sym. > > Yes, that’s a good point. I’ve been trying to separate the changes to > the functions from moving parts out but in this case it would make > just as much sense to add them to recordmcount.c in the first place. > > Ultimately, this ugliness gets removed as the next series removes > recordmcount.h entirely and one of the steps is moving > find_secsym_ndx() out while eliminating these redundant pieces.
Yeah, this code will be cleaned up later, but let's have the steps in between look fine as well.
> > > > > Another, probably more robust way of doing this, is change > > find_secsym_ndx() to return 0 on success and -1 on missing symbol, > > and just pass a pointer by reference to fill the recsym (which > > doesn't have to be a constant). > > That’s easy enough to do and I do like separating the error/success > return from returning the index. I can send that out now or tack it > onto the next RFC series I’m about to send which completes the > conversion if that’s preferable. > > Yeah, the original code applies “const” in lots of places -- I > presume it’s an attempt to eek out every last bit of performance from > the compiler.
As I said before, I've applied patches 1-3, so you don't need to resend them. I finished looking at the rest, and only this patch needs to be fixed, and since you are resending, could you fix the "upside-down x-mas" tree declaration I mentioned in patch 8.
Thanks Matt,
-- Steve
| |