[MPlayer-dev-eng] [PATCH] modify_reg.c

[Carl Eugen Hoyos]

Hi Reimar!

Reimar Döffinger wrote:
> Hello,
> On Sun, Aug 05, 2007 at 08:08:05PM +0200, Carl Eugen Hoyos wrote:
> [...]
>> +#define mp_msg(a, b, c, d, e)
>> +#include "get_path.c"
> 
> This is quite brittle, at least use the mp_msg stub that e.g.
> codec-cfg.c uses.

Fixed.

>> +    tmp_val = regs[i].value;
>> +    tmp_name = regs[i].name;
>> +    for(;i < reg_size -1; i++)
>> +        memcpy(&regs[i], &regs[i+1], sizeof(struct reg_value));
>> +    reg_size--;
>> +    free(tmp_val);
>> +    free(tmp_name);
> 
> What is the point of tmp_val and tmp_name, why not free them right away?
> And why the for-loop, why not memmove the whole block in one step?
> The same is done in hier_remove_key as well.

Removed unused remove_key, fixed hier_remove_key and renamed it to
remove_key.

>> +  strncpy(key, tmpkey + 1, strlen(tmpkey)-1);
>> +  key[strlen(tmpkey)-1] = 0;
> 
> Too bad av_strlcpy is not available... But maybe it's worth
> copy-and-pasting it?

Is this really necessary?

[...]

>> +	v = strtoul(value, NULL, 0);
> 
> Tab

Fixed.

New patch with license header and appropriate spaces attached.

Please review again, thank you, Carl Eugen

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patchreg
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20070806/ea9ec088/attachment.asc>