[MPlayer-dev-eng] [PATCH] misc small fixes (mostly gcc warnings)

[Rich Felker]

On Sat, Nov 12, 2005 at 10:46:07PM +0100, Dominik 'Rathann' Mierzejewski wrote:
> On Saturday, 12 November 2005 at 19:58, Rich Felker wrote:
> > On Sat, Nov 12, 2005 at 05:01:45PM +0100, Dominik 'Rathann' Mierzejewski wrote:
> > > -// is this needed? #include <sys/perm.h>
> > > +//#include <sys/perm.h> doesn't exist on libc5 systems, declare extern instead
> > > +extern int iopl(int level);
> > 
> > Return type is default and argument type does not need declaration in
> > C. It would be better to use:
> > 
> > int iopl();
> > 
> > in case kernel devs stupidly change int to unsigned or something in
> > the future. Or just omit the declaration entirely since it's totally
> > unnecessary.
> 
> Actually, according to man 2 iopl:
>        Libc5 treats it as a system call and has a prototype in <unistd.h>. 
>        Glibc1 does not have a prototype. Glibc2 has a prototype both in 
>        <sys/io.h> and in <sys/perm.h>. Avoid the latter, it is available
>        on i386 only.
> 
> So, if this is correct, this code may not even compile on a glibc1 system.

There's no such thing as a glibc1 system, is there? :)
Anyway I agree, sys/io.h is the correct include.

> > >  /* please upload RV10 samples WITH INDEX CHUNK */
> > > -static int demux_seek_real(demuxer_t *demuxer, float rel_seek_secs, int flags)
> > > +static void demux_seek_real(demuxer_t *demuxer, float rel_seek_secs, int flags)
> > >  {
> > >      real_priv_t *priv = demuxer->priv;
> > >      demux_stream_t *d_audio = demuxer->audio;
> > > @@ -1771,7 +1771,7 @@
> > >  //    printf("streams: %d\n", streams);
> > >  
> > >      if (!streams)
> > > -	return 0;
> > > +	return;
> > >  
> > >      if (flags & 1)
> > >  	/* seek absolute */
> > > @@ -1838,7 +1838,6 @@
> > >          stream_seek(demuxer->stream, next_offset);
> > >  
> > >      demux_real_fill_buffer(demuxer, NULL);
> > > -    return 1;
> > >  }
> > 
> > Huh? Changes like this are almost surely incorrect.. Before there was
> > a meaningful return value, now there is not. There are many more
> > mistakes like this too.
> 
> libmpdemux/demuxer.h:
> [...]
> /**
>  * Demuxer description structure
>  */
> typedef struct demuxers_desc_st {
> [...]
>   // Seek
>   void (*seek)(struct demuxer_st *demuxer, float rel_seek_secs, int flags); ///< Optional
> [...]
> } demuxer_desc_t;
> 
> Seek functions aren't supposed to return anything currently, so we should
> either change the above struct definition or fix all demuxer seek
> functions. I've done the latter, but perhaps the former is correct.

I think the former is correct. Before the demuxer modularization
overhaul, seek functions were intended to return a success/failure
code. If this was broken in the big patch, it needs to be fixed, not
further broken. But it should be a separate patch from the warning
cleanups.

> > In any case, this is NOT PART OF A WARNING FIX PATCH, but actual
> > functional changes. Do not mix them!!
> 
> OK.

:)

Rich