On 04/24, Jim Keniston wrote:
>
> I see a couple of nits in this patch (see below), but the others look
> good.
>
> Patches 1-5 of this set:
> Reviewed-by: Jim Keniston <[email protected]>
Thanks!
> > struct {
> > s32 offs;
> > u8 ilen;
> > u8 opc1;
> > - } branch;
> > + } branch;
> > + struct {
> > +#ifdef CONFIG_X86_64
> > + long riprel_target;
> > +#endif
> > + u16 fixups;
> > + } def;
>
> "def" is kind of ambiguous.
Heh. I am shy to admit that my plan was to name it "default". I changed
its name only after gcc told me I should learn "C".
> How about "dfault" or some such?
Then probably "dflt", this looks more consintent and more IBMish ;)
On a serious note, I agree with any naming... but "dfault" looks like
"d" fault to me. Perhaps something else?
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> ...
> > @@ -636,12 +635,12 @@ int arch_uprobe_analyze_insn(struct arch_uprobe
> > *auprobe, struct mm_struct *mm,
> >
> > /*
> > * Figure out which fixups arch_uprobe_post_xol() will need to perform,
> > - * and annotate arch_uprobe->fixups accordingly. To start with, ->fixups
> > - * is either zero or it reflects rip-related fixups.
> > + * and annotate def->fixups accordingly. To start with, ->fixups is
> > + * either zero or it reflects rip-related fixups.
>
> That sentence stopped being true a couple of patch sets ago.
> handle_riprel_insn() is called later in this function now.
Yes, but the comment mentions arch_uprobe_post_xol? Anyway, I'll update
it to say "default_post_xol_op" instead.
Or I misunderstood you ?
Oleg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
