[vlc-devel] [PATCH 02/11] demux: wav: refactor chunk skip

[Steve Lhomme]

On 2020-03-16 9:03, Rémi Denis-Courmont wrote:
> Le maanantaina 16. maaliskuuta 2020, 9.47.14 EET Steve Lhomme a écrit :
>> On 2020-03-13 16:53, Thomas Guillem wrote:
>>> Here is what I propose:
>>>
>>> static int ChunkSkip( demux_t *p_demux, uint32_t i_size )
>>> {
>>>
>>>       i_size += i_size & 1;
>>>       
>>>       if( i_size >= 65536 )
>>
>> In general constant values are bad in the code.
> 
> [disputed]
> 
> If the same value must be reused several times, I can see the point in that.
> (Though even then, we have plenty of, e.g., 80 instead of IPPORT_HTTP...)
> 
> But if the numebr is just an arbitrary limit that is only used once in the
> code base, I really don't see the point in adding a symbolic constant. That
> makes it confusingly look like it's actually a specified constant, which it is
> not.
> 
> Especially here where it's obvious enough what the surrounding code does.

That's precisely because I don't understand where this value comes from 
that I thought using a meaningful constant would help. Comments in the 
code could help too.