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

[Dominik 'Rathann' Mierzejewski]

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.
And <sys/perm.h> shouldn't even be used.

> > -  headers[0] = &extradata[offset];
> > -  headers[1] = &extradata[offset + hsizes[0]];
> > -  headers[2] = &extradata[offset + hsizes[0] + hsizes[1]];
> > +  headers[0] = (unsigned int *)&extradata[offset];
> > +  headers[1] = (unsigned int *)&extradata[offset + hsizes[0]];
> > +  headers[2] = (unsigned int *)&extradata[offset + hsizes[0] + hsizes[1]];
> 
> This just shows that headers[] is misdeclared. It should be declared
> as void*[3], not unsigned int *[3]. Fix the problem; don't add
> nonsensical casts.
> 
> > -    op.packet = headers[i];
> > +    op.packet = (unsigned char *)headers[i];
> 
> Same.

You're right. Will fix in next iteration.

> >  /* 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.

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

OK.

R.

-- 
MPlayer RPMs maintainer: http://rpm.greysector.net/mplayer/
"I am Grey. I stand between the candle and the star. We are Grey.
 We stand between the darkness ... and the light."
        -- Delenn in Grey Council in Babylon 5:"Babylon Squared"