On Friday, 1 May 2020 1:41:57 PM AEST Jordan Niethe wrote:
> create_branch(), create_cond_branch() and translate_branch() return the
> instruction that they create, or return 0 to signal an error. Separate
> these concerns in preparation for an instruction type that is not just
> an unsigned int. Fill the created instruction to a pointer passed as
> the first parameter to the function and use a non-zero return value to
> signify an error.
We're not adding any new checks for error cases here, but this patch doesn't
change existing behaviour as far as I can tell and adding checks would be
difficult (and could introduce other bugs) so would be best done as a separate
series anyway if required at all.
> @@ -403,6 +407,7 @@ static void __init test_trampoline(void)
>
> static void __init test_branch_iform(void)
> {
> + int err;
> unsigned int instr;
> unsigned long addr;
>
> @@ -443,35 +448,35 @@ static void __init test_branch_iform(void)
> check(instr_is_branch_to_addr(&instr, addr - 0x2000000));
>
> /* Branch to self, with link */
> - instr = create_branch(&instr, addr, BRANCH_SET_LINK);
> + err = create_branch(&instr, &instr, addr, BRANCH_SET_LINK);
> check(instr_is_branch_to_addr(&instr, addr));
The pointer alias above initially caught my eye, but it's ok because
create_branch() doesn't actually dereference the second instance. Arguably the
argument type could be changed to an unsigned long but then we'd just end up
with more casts so this is ok to me at least.
Reviewed-by: Alistair Popple <alist...@popple.id.au>
