[FFmpeg-devel] [PATCH 01/21] avformat/avio: Don't use incompatible function pointer type for call

[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.rheinhardt at 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).

Regards,
Marton