ext2spice.c mergeAttr() request for review

[dlmiles]

magic/ext2spice/ext2spice.c

Line 4259 in c4a2a54

t = (char *) strcat(*a1, *a2);

void
mergeAttr(a1, a2)
    char **a1, **a2;
{
    if (*a1 == NULL)
	*a1 = *a2;
    else
    {
	char *t;
	int l1 = strlen(*a1);
	int l2 = strlen(*a2);
	t = (char *) mallocMagic((unsigned int)((l1 + l2) + 1));
	t = (char *) strcat(*a1, *a2);  // PLEASE REVIEW THIS AREA
	freeMagic(*a1);
	*a1 = t;
    }
}

Consider:

	char *t;
	int l1 = strlen(*a1);
	int l2 = strlen(*a2);
	t = (char *) mallocMagic((unsigned int)((l1 + l2) + 1));
#if 0
 	t = (char *) strcat(*a1, *a2);
#else
	strcpy(t, *a1);
	strcat(t, *a2);
#endif
	freeMagic(*a1);
	*a1 = t;

[dlmiles]

Please also check this macro version of mergeAttr() of the same

https://github.com/RTimothyEdwards/magic/blob/master/ext2sim/ext2sim.c#L1758

Looks unused, maybe safer to delete ?

[RTimothyEdwards]

I agree the original version doesn't make any sense; the strcat() function merges string a2 into string a1 which does not necessarily have room to hold it, and the returns a pointer to string a1, causing the allocated memory t to be orphaned. I suggest fixing this as you have done above.

Likewise, I can find no place that calls mergeAttr() in ext2sim so I agree with your assessment of just removing it altogether.