On 8/13/2019 7:29 PM, Nicolas George wrote: > Juan De León (12019-08-13): >> The array is there so that the structure isn't opaque, it should be >> accessed with the function. > > I realize you need it, but not for the reason you say. It is needed for > alignment: if blocks needs more alignment than info, info+sizeof(info) > is not a valid pointer for blocks. > >>>> + if (!info || idx >= info->nb_blocks || idx < 0) >>>> + return NULL; >>> How valid is it for applications to call with idx outside the range? >> They shouldn't but I figure it's better to return NULL than to get >> undefined behaviour. > > In that case, I think is is better practice to document: > > Index must be between 0 and nb_blocks > > and check with an assert, forcing the application programmer fix their > code immediately. > > Most code will likely use idx from a loop counter, where it cannot be > outside the range, and for the few other cases, the caller can do the > check if necessary. > > Also, make the fields that cannot be negative unsigned, and you can drop > the <0 test. > > Regards,
I'm fairly sure this was discussed before, but invalid arguments shouldn't crash an user's application. They even have their own standardized errno value for this purpose. asserts() are to catch bugs in our code, not in theirs. Returning a NULL pointer is the preferred behavior. _______________________________________________ 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".