On 9/5/22 10:55, Milica Lazarevic wrote:
> -static std::string save_restore_list(uint64 rt, uint64 count, uint64 gp)
> +static char *save_restore_list(uint64 rt, uint64 count, uint64 gp)
> {
> - std::string str;
> + /*
> + * Currently, this file compiles as a cpp file, so the explicit cast here
> + * is necessary. Later, the cast will be removed.
> + */
> + char *str = (char *)g_malloc(200);
> + str[0] = '\0';
>
> 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;
> - str += img_format(",%s", GPR(this_rt));
> + strcat(str, img_format(",%s", GPR(this_rt)));
> }
>
> return str;
> }
This would be better written as
char *reg_list[33];
assert(count <= 32);
for (c = 0; c < count; c++) {
bool use_gp = ...
uint64 this_rt = ...
/* glib usage below requires casting away const */
reg_list[c] = (char *)GPR(this_rt);
}
reg_list[count] = NULL;
return g_strjoinv(",", reg_list);
In the implementation you suggested, there's one comma missing in the output.
For example, instead of having:
> 0x802021ce: 1e12 SAVE 0x10,ra,s0
We're having this:
< 0x802021ce: 1e12 SAVE 0x10ra,s0
So, I'm assuming that there needs to exist one more concatenation between the
comma and the result of the g_strjoinv function?
Maybe something like
return g_strconcat((char *)",", (char *)g_strjoinv(",", reg_list), NULL);
this?
Would it be acceptable to go with a similar implementation as in the patch, but
without a call to img_format and with the g_strconcat instead of the strcat
function?
> @@ -716,7 +617,7 @@ static uint64 extract_op_code_value(const uint16 *data,
> int size)
> * instruction size - negative is error
> * disassembly string - on error will constain error string
> */
> -static int Disassemble(const uint16 *data, std::string & dis,
> +static int Disassemble(const uint16 *data, char *dis,
I think this interface should be
char **dis,
so that...
> @@ -746,25 +647,26 @@ static int Disassemble(const uint16 *data, std::string
> & dis,
> * an ASE attribute and the requested
> version
> * not having that attribute
> */
> - dis = "ASE attribute mismatch";
> + strcpy(dis, "ASE attribute mismatch");
these become
*dis = g_strdup("string");
and the usage in nanomips_dis does not assume a fixed sized buffer.
r~
I'm not sure why the fixed size buffer would be a problem here since the buffer
size has already been limited by the caller.
I.e. in the print_insn_nanomips function, the buf variable is defined as:
char buf[200];
