Re: [FFmpeg-devel] [PATCH 1/3] avformat/file: check for lseek() failure

["Rémi Denis-Courmont" <remi@remlab.net>]

Le lauantaina 25. toukokuuta 2024, 14.18.12 EEST Michael Niedermayer a écrit :
> Fixes: CID1559855 Unchecked return value from library

This is again very misleading. Logging an error (that most people won't even 
see) is not fixing a bug. Either this is not a bug, and this patch can't be 
fixing a non-bug, or this is a bug, and this patch is hiding it behind a fake 
fix.

In this case, the code looks completely bogus. Restoring the file pointer 
before closing the descriptor makes no sense. In the first place, it can only 
have an observable effect if the file descriptor is borrowed (i.e. "fd" 
protocol). And even then, whatever external code relies on this peculiar 
behaviour is irremediably broken, as Coverity highlights. If this is really 
needed, the only way that this can work properly is for that external code to 
restore the file offset after libavformat is done, and check the error *there*.

Also, AFAIK, by the principle of least surprise, and following the behaviour 
of most other programs in similar circumstances, the expected behaviour is 
*not* to restore the file pointer.

So IMO, 5c3d2177e7be7dde69a83b1b394f57976c164c84 should be reverted instead.

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".