On 3/25/2021 8:55 AM, Nicolas George wrote:
James Almer (12021-03-24):
I think it's clear by now that nothing i could say will convince you it's
better to not return a pointer to an internal array when there are safer
alternatives, and i already gave my reasons why, none of which satisfied
you, so i don't see the point in keeping this discussion going.
I find this comment quite offensive. You did not manage to convince me
because your arguments have not been up to the task. Do not try to push
the fault on me, and I will refrain from accusing you of not taking my
arguments into account. Coming to an agreement is a process, it requires
both parts to refine their arguments progressively.
It was not meant to be offensive. As i said above i have no other
argument against your approach to this function than what i already
said, hence why anything i might add will be more or less repeating myself.
This is a matter of choosing the least of several drawbacks. So let us
compare the drawbacks and not muddle things further.
For me:
1. having a dynamic allocation is way way worse than
2. having sizeof(AVIndexEntry) in the ABI, which is somewhat worse than
3. having a function with many arguments, which is a tiny bit worse than
4. having a "use this pointer immediately" constraint.
We agree except on 3>4, so let us focus on that.
Option (3) has these practical drawbacks: Many arguments involves more
typing and the risk of messing with the order and getting invalid
values. It also requires redesigning the API if we add fields and
exporting them is useful. And it requires either the overhead of NULL
checks or the caller declaring unneeded variables.
Same situation as av_add_index_entry(). And if you look at the signature
of the (3) function in my last proposal, i kept the av_index_* namespace
free precisely in case a new API in the future needs to be added for
this reason, for both add() and get(). One that could work directly with
the AVIndexEntry struct after the internal entries array was redesigned,
perhaps using AVTreeNode, so returned pointers or entries are safer to
handle.
Option (4) has the obvious practical drawback that misusing the API
causes undefined behavior.
The constraint of using a pointer immediately on risk of undefined
behavior is actually a frequent one, in FFmpeg but also in C at large:
gethosbtyname, localtime, etc.
For me, that makes it approximately on par with the risk of messing the
order of the many arguments.
Which leaves more typing, NULL checks overhead or useless variables
(still more typing).
It is tiny, I have no trouble admitting, but it is tiny in favor of one
solution.
If you do not agree with these estimates, please explain exactly where.
I don't know if you remember how in this one imgutils patch i sent some
time ago i was against functions with tons of output parameters, because
i considered it ugly and typical of enterprise software API. That hasn't
changed. But here, being the exact counterpart of an existing add()
function put it above the other approach i dislike slightly more of
returning an internal pointer and not being able to tell the user just
what may invalidate it.
If some other developer wants to chime in and comment which approach they
prefer, then that would be ideal.
Indeed.
Regards,
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
