[vlc-devel] [PATCH] demux/mpeg: ts_bsearch_searchkey_Compare: fix warning

[Filip Roséen]

Hi Rémi,

On 2017-05-18 19:05, Rémi Denis-Courmont wrote:

> Le torstaina 18. toukokuuta 2017, 12.19.37 EEST Filip Roséen a écrit :
> > The address of this function is passed to bsearch, and bsearch accepts
> > a pointer to int(void const*,void const*), meaning that the previous
> > prototype would generate a warning (and strictly speaking was not
> > valid).
> > ---
> >  modules/demux/mpeg/ts_pid.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/modules/demux/mpeg/ts_pid.c b/modules/demux/mpeg/ts_pid.c
> > index 27779a04d8..eed60a3a28 100644
> > --- a/modules/demux/mpeg/ts_pid.c
> > +++ b/modules/demux/mpeg/ts_pid.c
> > @@ -65,7 +65,7 @@ struct searchkey
> >      ts_pid_t **pp_last;
> >  };
> > 
> > -static int ts_bsearch_searchkey_Compare( void *key, void *other )
> > +static int ts_bsearch_searchkey_Compare( const void *key, const void *other
> > ) {
> >      struct searchkey *p_key = (struct searchkey *) key;
> >      ts_pid_t *p_pid = *((ts_pid_t **) other);
> 
> Removing the const qualifier via cast is not a good practice, even if it is 
> legal.

I agree, and while we are talking about that function I realized that
I forgot to send a follow-up email that talks about how it is doing a
lot of things that it is not supposed to.

For one, as can be seen by [this part of the implementation][1], it is
making assumptions about the implementation of `bsearch` that are not
guaranteed to be true in practice - at least I do not know what
wording in the standard that states that the search should stop at
what would be the insertion point.

I am not sure if a ticket should be created about it, or if this email
is sufficient enough to actually warrant a fixup of the actual code in
question. Honestly, I was too tired to fix it while I fixed the
mentioned diagnostic (its the only reason I didn't do it as part of
the commit).

[1]: http://git.videolan.org/?p=vlc.git;a=blob;f=modules/demux/mpeg/ts_pid.c;h=27779a04d814b78cb675e37ebfd017b53d96c3e5;hb=HEAD#l134

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170518/39fcf60b/attachment-0001.html>