Re: [PATCH] powerpc/xmon: Fix comparing pointer
From: Christophe Leroy
Date: Tue Jun 13 2023 - 02:06:39 EST
Le 13/06/2023 à 05:38, wuyonggang001@xxxxxxxxxx a écrit :
>
>
> Fix the following coccicheck warning:
>
> arch/powerpc/xmon/spu-dis.c:51:34-35: WARNING comparing pointer to 0
Please send text messages, not html.
And please check your patch, a lot of lines have no modifications but
appears as modified in your patch because you changed the formating.
Please don't change more than what is intended to change.
Thanks
>
> Signed-off-by: Yonggang Wu <wuyonggang001@xxxxxxxxxx>
> ---
> arch/powerpc/xmon/spu-dis.c | 384 ++++++++++++++++++------------------
> 1 file changed, 193 insertions(+), 191 deletions(-)
>
> diff --git a/arch/powerpc/xmon/spu-dis.c b/arch/powerpc/xmon/spu-dis.c
> index 4b0a4e640f08..f48a2ddd7440 100644
> --- a/arch/powerpc/xmon/spu-dis.c
> +++ b/arch/powerpc/xmon/spu-dis.c
> @@ -22,216 +22,218 @@ extern const int spu_num_opcodes;
> #define SPU_DISASM_TBL_SIZE (1 << 11)
> static const struct spu_opcode
> *spu_disassemble_table[SPU_DISASM_TBL_SIZE];
>
> -static void
> -init_spu_disassemble (void)
> +static void init_spu_disassemble(void)
> {
> - int i;
> -
> - /* If two instructions have the same opcode then we prefer the first
> - * one. In most cases it is just an alternate mnemonic. */
> - for (i = 0; i < spu_num_opcodes; i++)
> - {
> - int o = spu_opcodes[i].opcode;
> - if (o >= SPU_DISASM_TBL_SIZE)
> - continue; /* abort (); */
> - if (spu_disassemble_table[o] == 0)
> - spu_disassemble_table[o] = &spu_opcodes[i];
> - }
> + int i;
> +
> + /*
> + * If two instructions have the same opcode then we prefer the first
> + * one. In most cases it is just an alternate mnemonic.
> + */
> + for (i = 0; i < spu_num_opcodes; i++) {
> + int o = spu_opcodes[i].opcode;
> +
> + if (o >= SPU_DISASM_TBL_SIZE)
> + continue; /* abort(); */
> + if (spu_disassemble_table[o] == NULL)
> + spu_disassemble_table[o] = &spu_opcodes[i];
> + }
> }
>
> /* Determine the instruction from the 10 least significant bits. */
> -static const struct spu_opcode *
> -get_index_for_opcode (unsigned int insn)
> +static const struct spu_opcode *get_index_for_opcode(unsigned int insn)
> {
> - const struct spu_opcode *index;
> - unsigned int opcode = insn >> (32-11);
> -
> - /* Init the table. This assumes that element 0/opcode 0 (currently
> - * NOP) is always used */
> - if (spu_disassemble_table[0] == 0)
> - init_spu_disassemble ();
> -
> - if ((index = spu_disassemble_table[opcode & 0x780]) != 0
> - && index->insn_type == RRR)
> - return index;
> -
> - if ((index = spu_disassemble_table[opcode & 0x7f0]) != 0
> - && (index->insn_type == RI18 || index->insn_type == LBT))
> - return index;
> -
> - if ((index = spu_disassemble_table[opcode & 0x7f8]) != 0
> - && index->insn_type == RI10)
> - return index;
> -
> - if ((index = spu_disassemble_table[opcode & 0x7fc]) != 0
> - && (index->insn_type == RI16))
> - return index;
> -
> - if ((index = spu_disassemble_table[opcode & 0x7fe]) != 0
> - && (index->insn_type == RI8))
> - return index;
> -
> - if ((index = spu_disassemble_table[opcode & 0x7ff]) != 0)
> - return index;
> -
> - return NULL;
> + const struct spu_opcode *index;
> + unsigned int opcode = insn >> (32-11);
> +
> + /*
> + * Init the table. This assumes that element 0/opcode 0 (currently
> + * NOP) is always used
> + */
> + if (spu_disassemble_table[0] == NULL)
> + init_spu_disassemble();
> +
> + index = spu_disassemble_table[opcode & 0x780];
> + if (index != NULL && index->insn_type == RRR)
> + return index;
> +
> + index = spu_disassemble_table[opcode & 0x7f0];
> + if (index != NULL
> + && (index->insn_type == RI18 || index->insn_type == LBT))
> + return index;
> +
> + index = spu_disassemble_table[opcode & 0x7f8];
> + if (index != NULL
> + && index->insn_type == RI10)
> + return index;
> +
> + index = spu_disassemble_table[opcode & 0x7fc]
> + if (index != NULL && (index->insn_type == RI16))
> + return index;
> +
> + index = spu_disassemble_table[opcode & 0x7fe];
> + if (index != NULL && (index->insn_type == RI8))
> + return index;
> +
> + index = spu_disassemble_table[opcode & 0x7ff];
> + if (index != NULL)
> + return index;
> +
> + return NULL;
> }
>
> /* Print a Spu instruction. */
>
> -int
> -print_insn_spu (unsigned long insn, unsigned long memaddr)
> +int print_insn_spu(unsigned long insn, unsigned long memaddr)
> {
> - int value;
> - int hex_value;
> - const struct spu_opcode *index;
> - enum spu_insns tag;
> + int value;
> + int hex_value;
> + const struct spu_opcode *index;
> + enum spu_insns tag;
>
> - index = get_index_for_opcode (insn);
> + index = get_index_for_opcode(insn);
>
> - if (index == 0)
> - {
> - printf(".long 0x%lx", insn);
> - }
> - else
> - {
> - int i;
> - int paren = 0;
> - tag = (enum spu_insns)(index - spu_opcodes);
> - printf("%s", index->mnemonic);
> - if (tag == M_BI || tag == M_BISL || tag == M_IRET || tag == M_BISLED
> - || tag == M_BIHNZ || tag == M_BIHZ || tag == M_BINZ || tag == M_BIZ
> - || tag == M_SYNC || tag == M_HBR)
> + if (index == NULL)
> {
> - int fb = (insn >> (32-18)) & 0x7f;
> - if (fb & 0x40)
> - printf(tag == M_SYNC ? "c" : "p");
> - if (fb & 0x20)
> - printf("d");
> - if (fb & 0x10)
> - printf("e");
> - }
> - if (index->arg[0] != 0)
> - printf("\t");
> - hex_value = 0;
> - for (i = 1; i <= index->arg[0]; i++)
> + printf(".long 0x%lx", insn);
> + } else
> {
> - int arg = index->arg[i];
> - if (arg != A_P && !paren && i > 1)
> - printf(",");
> -
> - switch (arg)
> - {
> - case A_T:
> - printf("$%lu",
> - DECODE_INSN_RT (insn));
> - break;
> - case A_A:
> - printf("$%lu",
> - DECODE_INSN_RA (insn));
> - break;
> - case A_B:
> - printf("$%lu",
> - DECODE_INSN_RB (insn));
> - break;
> - case A_C:
> - printf("$%lu",
> - DECODE_INSN_RC (insn));
> - break;
> - case A_S:
> - printf("$sp%lu",
> - DECODE_INSN_RA (insn));
> - break;
> - case A_H:
> - printf("$ch%lu",
> - DECODE_INSN_RA (insn));
> - break;
> - case A_P:
> - paren++;
> - printf("(");
> - break;
> - case A_U7A:
> - printf("%lu",
> - 173 - DECODE_INSN_U8 (insn));
> - break;
> - case A_U7B:
> - printf("%lu",
> - 155 - DECODE_INSN_U8 (insn));
> - break;
> - case A_S3:
> - case A_S6:
> - case A_S7:
> - case A_S7N:
> - case A_U3:
> - case A_U5:
> - case A_U6:
> - case A_U7:
> - hex_value = DECODE_INSN_I7 (insn);
> - printf("%d", hex_value);
> - break;
> - case A_S11:
> - print_address(memaddr + DECODE_INSN_I9a (insn) * 4);
> - break;
> - case A_S11I:
> - print_address(memaddr + DECODE_INSN_I9b (insn) * 4);
> - break;
> - case A_S10:
> - case A_S10B:
> - hex_value = DECODE_INSN_I10 (insn);
> - printf("%d", hex_value);
> - break;
> - case A_S14:
> - hex_value = DECODE_INSN_I10 (insn) * 16;
> - printf("%d", hex_value);
> - break;
> - case A_S16:
> - hex_value = DECODE_INSN_I16 (insn);
> - printf("%d", hex_value);
> - break;
> - case A_X16:
> - hex_value = DECODE_INSN_U16 (insn);
> - printf("%u", hex_value);
> - break;
> - case A_R18:
> - value = DECODE_INSN_I16 (insn) * 4;
> - if (value == 0)
> - printf("%d", value);
> - else
> + int i;
> + int paren = 0;
> +
> + tag = (enum spu_insns)(index - spu_opcodes);
> +
> + printf("%s", index->mnemonic);
> + if (tag == M_BI || tag == M_BISL || tag == M_IRET || tag ==
> M_BISLED
> + || tag == M_BIHNZ || tag == M_BIHZ || tag == M_BINZ || tag == M_BIZ
> + || tag == M_SYNC || tag == M_HBR)
> {
> - hex_value = memaddr + value;
> - print_address(hex_value & 0x3ffff);
> + int fb = (insn >> (32-18)) & 0x7f;
> +
> + if (fb & 0x40)
> + printf(tag == M_SYNC ? "c" : "p");
> + if (fb & 0x20)
> + printf("d");
> + if (fb & 0x10)
> + printf("e");
> }
> - break;
> - case A_S18:
> - value = DECODE_INSN_U16 (insn) * 4;
> - if (value == 0)
> - printf("%d", value);
> - else
> - print_address(value);
> - break;
> - case A_U18:
> - value = DECODE_INSN_U18 (insn);
> - if (value == 0 || 1)
> + if (index->arg[0] != 0)
> + printf("\t");
> + hex_value = 0;
> + for (i = 1; i <= index->arg[0]; i++) {
> + int arg = index->arg[i];
> +
> + if (arg != A_P && !paren && i > 1)
> + printf(",");
> +
> + switch (arg) {
> + case A_T:
> + printf("$%lu",
> + DECODE_INSN_RT(insn));
> + break;
> + case A_A:
> + printf("$%lu",
> + DECODE_INSN_RA(insn));
> + break;
> + case A_B:
> + printf("$%lu",
> + DECODE_INSN_RB(insn));
> + break;
> + case A_C:
> + printf("$%lu",
> + DECODE_INSN_RC(insn));
> + break;
> + case A_S:
> + printf("$sp%lu",
> + DECODE_INSN_RA(insn));
> + break;
> + case A_H:
> + printf("$ch%lu",
> + DECODE_INSN_RA(insn));
> + break;
> + case A_P:
> + paren++;
> + printf("(");
> + break;
> + case A_U7A:
> + printf("%lu",
> + 173 - DECODE_INSN_U8(insn));
> + break;
> + case A_U7B:
> + printf("%lu",
> + 155 - DECODE_INSN_U8(insn));
> + break;
> + case A_S3:
> + case A_S6:
> + case A_S7:
> + case A_S7N:
> + case A_U3:
> + case A_U5:
> + case A_U6:
> + case A_U7:
> + hex_value = DECODE_INSN_I7(insn);
> + printf("%d", hex_value);
> + break;
> + case A_S11:
> + print_address(memaddr + DECODE_INSN_I9a(insn) * 4);
> + break;
> + case A_S11I:
> + print_address(memaddr + DECODE_INSN_I9b(insn) * 4);
> + break;
> + case A_S10:
> + case A_S10B:
> + hex_value = DECODE_INSN_I10(insn);
> + printf("%d", hex_value);
> + break;
> + case A_S14:
> + hex_value = DECODE_INSN_I10(insn) * 16;
> + printf("%d", hex_value);
> + break;
> + case A_S16:
> + hex_value = DECODE_INSN_I16(insn);
> + printf("%d", hex_value);
> + break;
> + case A_X16:
> + hex_value = DECODE_INSN_U16(insn);
> + printf("%u", hex_value);
> + break;
> + case A_R18:
> + value = DECODE_INSN_I16(insn) * 4;
> + if (value == 0)
> + printf("%d", value);
> + else {
> + hex_value = memaddr + value;
> + print_address(hex_value & 0x3ffff);
> + }
> + break;
> + case A_S18:
> + value = DECODE_INSN_U16(insn) * 4;
> + if (value == 0)
> + printf("%d", value);
> + else
> + print_address(value);
> + break;
> + case A_U18:
> + value = DECODE_INSN_U18(insn);
> + if (value == 0 || 1) {
> + hex_value = value;
> + printf("%u", value);
> + } else
> + print_address(value);
> + break;
> + case A_U14:
> + hex_value = DECODE_INSN_U14(insn);
> + printf("%u", hex_value);
> + break;
> + }
> + if (arg != A_P && paren)
> {
> - hex_value = value;
> - printf("%u", value);
> + printf(")");
> + paren--;
> }
> - else
> - print_address(value);
> - break;
> - case A_U14:
> - hex_value = DECODE_INSN_U14 (insn);
> - printf("%u", hex_value);
> - break;
> - }
> - if (arg != A_P && paren)
> - {
> - printf(")");
> - paren--;
> - }
> }
> - if (hex_value > 16)
> - printf("\t# %x", hex_value);
> + if (hex_value > 16)
> + printf("\t# %x", hex_value);
> }
> return 4;
> }