Messages in this thread | | | Date | Sat, 14 Feb 2009 10:20:39 -0500 (EST) | From | Steven Rostedt <> | Subject | Re: [PATCH 2/2] powerpc, ftrace: use create_branch lib function |
| |
On Sun, 15 Feb 2009, Michael Ellerman wrote: > > > > /* if (link) set op to 'bl' else 'b' */ > > - op = 0x48000000 | (link ? 1 : 0); > > - op |= (ftrace_calc_offset(ip, addr) & 0x03fffffc); > > + op = create_branch((unsigned int *)ip, addr, link ? 1 : 0); > > If I was feeling nit-picky I'd say you should use: > > op = create_branch((unsigned int *)ip, addr, link ? BRANCH_SET_LINK : 0); > > > But admittedly we're unlikely to ever change the flag handling, so it's > probably not worth the effort of a respin (or this email :).
Not worth the effort of a respin, I agree.
But it would be worth the effort of a clean up patch ;-)
Not for "oh this flag might change", but for reviewers looking at the code and knowing exactly what that '1' means.
-- Steve
| |