[FFmpeg-devel] [PATCH 1/4] lavu: add simple array implementation
george at nsup.org
Wed Mar 5 19:34:35 CET 2014
Le quintidi 15 ventôse, an CCXXII, wm4 a écrit :
> Unfortunately, this is pretty horrible. GET_UTF8 style.
That is your personal taste, it is not corroborated by any argument.
And the beauty of that kind of API is: if you don't like it, don't use it.
> It could be simplified a bit. First off, you don't need to put the code
> for "success" as macro parameter. You could call the macro something
> like AV_DYNARRAY_GROW, and put the "success" code after it.
That means the failure case has to be some kind a jump, or it needs an extra
condition. That seems to make the macro less usable for no apparent reason
except alleged "simplification".
> Second, you could drop the type for the size. You'll only ever use
> size_t or int as index, so it's better to duplicate the macro for those
> two types, instead of wasting 2 macro parameters on them.
Until someone is forced by some reason or another to use a variable of
another type, for example to interact with a library that uses long.
It is this kind of lack of foresight that leads to having two
implementations where there could be just one for the start.
Also, a type parameter does not cost anything in a macro, there is just no
reason to omit it.
On the other hand, the type may not be actually necessary: it would probably
only be used to create a temp variable for the size, and we can use size_t
unconditionally for that.
> Third: are you sure you need the element type? In most situations, you
> can probably just use sizeof(array) inside the macro to get the size
> of the element.
That would mean type-pruning the array and doing the pointer arithmetic by
hand. It is usually safer to let the compiler do the arithmetic and print
warning about type mismatch if necessary.
Of course, it would simplify everything if we could use typeof; I would not
object to having two versions, one using typeof and clean pointer arithmetic
and one using sizeof and ugly pointer arithmetic. But in a public header
that is probably not a good idea.
For that issue, I would conclude: let us not discuss without actually seeing
> That leaves error handling. I'm not sure how to do that. Passing a
> statement as macro parameter is really ugly.
Once again: a matter of taste not corroborated by any argument.
> Maybe pass only the label
> and hard code the goto? IMHO slightly better, but not really great
IMO, infinitely worse.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 819 bytes
Desc: Digital signature
More information about the ffmpeg-devel