On date Friday 2025-04-25 13:16:59 +0000, softworkz . wrote:
[...]
> Tell me what I should check for and what not in those four groups of
> functions and for those things which should be checked, tell me which
> way (return error, return silently, allow segfault or use an assertion).
>
> Then I'll apply that to all those functions in a uniform and consistent
> way without even arguing and the case is closed.
>
> I just don't want to leave it alone like now without clear patterns,
> that's all 😊
I don't really have an answer. Probably it's good to start from the
docs, so that we have a definition of the semantics in advance, for
example stating that a pointer should not be NULL and so on so that at
least we know what is to be considered undefined behavior. As noted by
Nicolas, the pattern is dependant on the function behavior and on
practical ergonomy considerations.
It also would be nice to have a good set of guidelines.
Consider the case of a function which writes to a file:
int avbikeshed_write_color_to_file(const char *filename, const char *color)
It might fail in several ways: filename or color might be NULL,
filename might be missing or be a directory, the user might miss
permissions, the color might not be valid (assuming we want to parse
it).
We might want to assume that filename and color are non-NULL, enabling
the function to crash - for this it migth be good to add an assert and
usually this is the responsibility of the programmer to check for
this. While we want to log feedback and enable the program to handle
the case of invalid file/permissions or invalid color.
The pattern here is: assert/crash for NULL pointers (unless it makes
sense to have a NULL pointer), log and return feedback in case of
invalid input which is validated from within the function.
The other case in the API was:
int avbikeshed_add_color_slice(AVBikeshed *bikeshed, int level)
assuming this can only get a limited number of slices
(MAX_SLICE_COUNT, MAX_SLICE_LEVEL).
This might fail in several ways: bikeshed might be NULL or invalid
(e.g. a pointer to an unrelated structure), level might be invalid
(e.g. negative or >MAX_SLICE_LEVEL) or the bikeshed might contain
already too many slices.
The level might be checked by the programmer, so we might decide to
have an assert. About the count check it is validated from within the
function (since we need to access the bikeshed context) so we want to
provide feedback and fail.
For both of these two examples, doing nothing does not seem a good
idea. That's probably only good if we want to enable idem-potency or
when one of the parameter can be interpreted as a "none" argument.
For example:
if (color == NULL) {
return 0;
}
In this case we should specify the behavior in the documentation,
since that defines what is the undefined behavior and the input
expectactions.
_______________________________________________
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".
