On Sun, 10 Sep 2023, Andreas Rheinhardt wrote:
Marton Balint:
On Sat, 9 Sep 2023, Tomas Härdin wrote:
fre 2023-09-08 klockan 22:38 +0200 skrev Marton Balint:
On Thu, 7 Sep 2023, Andreas Rheinhardt wrote:
> It is undefined behaviour even in cases where it works
> (it works because it is only a const uint8_t* vs. uint8_t*
> difference).
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
> ---
> libavformat/avio.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
> > diff --git a/libavformat/avio.c b/libavformat/avio.c
> index ab1c19a58d..d53da5cb0c 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -354,10 +354,15 @@ fail:
> }
> > static inline int retry_transfer_wrapper(URLContext *h, uint8_t
> *buf,
> + const uint8_t *cbuf,
> int size, int size_min,
> - int
> (*transfer_func)(URLContext *h,
> -
> uint8_t *buf,
> - int
> size))
> + int
> (*read_func)(URLContext *h,
> + uint8_t
> *buf,
> + int
> size),
> + int
> (*write_func)(URLContext *h,
> + const
> uint8_t *buf,
> + int
> size),
> + int read)
These extra parameters are very ugly, can't we think of another way
to properly support this?
One idea is putting retry_transfer_wrapper in a template file and
include it twice with proper defines-s for the read and write flavours.
Seems like a lot of work for a function that's internal to avio.c
If future extensibility is not important here then function pointers
should not be passed to retry_tranfer_wrapper because
h->prot->url_read/write can be used directly. And usage of buf/cbuf is
readundant with the read paramter, because by checking if buf or cbuf is
null you can decide the operation (read of write).
The compiler does not know whether buf given to
ffurl_(read|write|read_complete) is NULL or not in the first place (it
also does not know whether the url_read and url_write function pointers
are NULL or not); therefore if one use e.g. cbuf != NULL as meaning read
== 0, then the write function would actually check for whether cbuf is
NULL which is worse than it is now.
(My initial version (not sent to this list) checked for whether the read
function was NULL in order to determine whether we are reading or
writing; the assumption was that the compiler would optimize the check
away when reading, because if the read function were NULL, then a NULL
function pointer would be used for a call, which is undefined behaviour.
But it didn't. Instead it added ffurl_read.cold and
ffurl_read_complete.cold functions (which presumably abort or so) for
this case.)
Maybe this could work to make the compiler optimize away the undeeded one:
if (buf && !cbuf)
write();
if (!buf && cbuf)
read();
But v2 is also fine, use whichever you prefer.
Thanks,
Marton
_______________________________________________
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".
