Messages in this thread | | | Date | Tue, 24 Jan 2023 14:32:41 -0800 | From | Josh Poimboeuf <> | Subject | Re: [PATCH v9] livepatch: Clear relocation targets on a module removal |
| |
On Tue, Jan 24, 2023 at 03:08:27PM -0500, Joe Lawrence wrote: > > + /* > > + * For a livepatch relocation, the restore r2 instruction might have > > + * been previously written if the relocation references a symbol in a > > + * module which was unloaded and is now being reloaded. In that case, > > + * skip the warning and instruction write. > > + */ > > + if (klp_sym && insn_val == PPC_INST_LD_TOC) > > + return 0; > > Hi Josh, > > Nit: shouldn't this return 1? > > And if you're willing to entertain a small refactor, wouldn't > restore_r2() be clearer if it returned -ESOMETHING on error? > > Maybe converting to a boolean could work, but then I'd suggest a name > that clearly implies success/fail given true/false return. Maybe > replace_nop_with_ld_toc() or replace_nop_to_restore_r2() ... still > -ESOMETHING is more intuitive to me as there are cases like this where > the function safely returns w/o replacing anything.
Indeed, and I actually already discovered that and made such changes, just need to get around to posting the patches.
-- Josh
| |