Re: [PATCH v2 13/20] disas/nanomips: Prevent memory leaking

[Richard Henderson]

On 9/5/22 10:55, Milica Lazarevic wrote:
> g_autofree attribute is added for every dynamically allocated string to
> prevent memory leaking.
>
> The implementation of the several functions that work with dynamically
> allocated strings is slightly changed so we can add those attributes.
>
> Almost every disassembly_function returns the result of the img_format()
> function, which returns a dynamically allocated string. To be able to
> free that string for every disassembly_function, a strdup() call is
> added for a return value of some disassembly functions like TLBGINV,
> TLBGINVF, TLBGP, etc.
>
> Signed-off-by: Milica Lazarevic <milica.lazarevic@syrmia.com>
> ---
>   disas/nanomips.cpp | 714 +++++++++++++++++++++++----------------------
>   1 file changed, 361 insertions(+), 353 deletions(-)
>
> diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
> index cfea95130d..473c202649 100644
> --- a/disas/nanomips.cpp
> +++ b/disas/nanomips.cpp
> @@ -523,7 +523,8 @@ static char *save_restore_list(uint64 rt, uint64 count, 
> uint64 gp)
>       for (uint64 counter = 0; counter != count; counter++) {
>           bool use_gp = gp && (counter == count - 1);
>           uint64 this_rt = use_gp ? 28 : ((rt & 0x10) | (rt + counter)) & 0x1f;
> -        strcat(str, img_format(",%s", GPR(this_rt)));
> +        g_autofree char *dis_str = img_format(",%s", GPR(this_rt));
> +        strcat(str, dis_str);
>       }

Will be unnecessary with the g_strjoinv change I suggested.


> @@ -657,7 +658,8 @@ static int Disassemble(const uint16 *data, char *dis,
>                                   return -6;
>                               }
>                               type = table[i].type;
> -                            strcpy(dis, dis_fn(op_code, m_pc));
> +                            g_autofree char *dis_str = dis_fn(op_code, m_pc);
> +                            strcpy(dis, dis_str);
>                               return table[i].instructions_size;

Will be unnecessary with the "*dis = value" return I suggested.

> @@ -1727,8 +1729,8 @@ static char *ACLR(uint64 instruction, img_address m_pc)
>       uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
>       int64 s_value = extract_s__se8_15_7_6_5_4_3_2_1_0(instruction);
>   
> -    char *bit = IMMEDIATE(copy(bit_value));
> -    char *s = IMMEDIATE(copy(s_value));
> +    g_autofree char *bit = IMMEDIATE(copy(bit_value));
> +    g_autofree char *s = IMMEDIATE(copy(s_value));
>       const char *rs = GPR(copy(rs_value));
>   
>       return img_format("ACLR %s, %s(%s)", bit, s, rs);

All of these should be switched to directly use printf format on the integer values, now 
that's supported by img_format.  That should be done as a separate patch, immediately 
before this one.


r~