[MPlayer-dev-eng] Re: [PATCH] loader compiler warnings

[Dominik 'Rathann' Mierzejewski]

On Friday, 09 April 2004 at 00:04, adland wrote:
> > > 
> > > see wine code
> > > in dlls/kernel/path.c function is defined as
> > > DWORD WINAPI GetFullPathNameA( LPCSTR name, DWORD len,
> > > LPSTR buffer, LPSTR *lastpart )
> > 
> > It doesn't matter what's in wine. Our loader comes from avifile.
> 
> this function to my knowledge is only called by external win32 dll files.
> why would you not want to match wine (which avifile loader was based on).

Because we're syncing with avifile, not with wine.

> > IMHO it's unacceptable, because it actually changes/removes loader code.
> 
> your previous statement was confusing. see below
> 
> [quote]
> >This is our code (not in avifile's loader), so we may mess with it. ;)

Yes. There's some additional code (i.e. to support QuickTime DLLs), which
is not in avifile. I don't see a problem changing that code, because it
may produce rejects when syncing with avifile anyway.

> now I have downloaded and looked at avifile source for this file
> avifile/plugins/libwin32/loader/win32.c
> 
> I see that function does exist here as well with same old format.

But different from what you propose:
avifile/plugins/libwin32/loader/win32.c:3034
static DWORD WINAPI expGetFullPathNameA
(
        LPCTSTR lpFileName,
        DWORD nBufferLength,
        LPTSTR lpBuffer,
        LPTSTR lpFilePart
){

> looks like we would want that they are also syncing from us 
> if we have changes to file.

Then send your patch to avifile maintainer. If it's accepted, then we
can talk about changes in the same code in MPlayer.

> note I have submitted several patches with changes to this file
>  which I have no idea if they are synced in avifile or not.
> 
> > Remember, our loader comes from avifile, so any change to original code
> > means rejected hunks when syncing with avifile's loader. Besides, your
> > patch does basically the same as mine, so what's the improvement?
> 
> yes it is simmilar. 
> 
> small difference
> -matching the wine/windows format for this function.

As I said: as I understand, we don't care what's in wine. Our (and
avifile's) loader was forked from wine a long time ago.

> -removing some old #if 0 code which is not needed

But the #if 0 code is the original avifile loader code, so it shouldn't
be removed.

> please comment

Yes, a comment from Sascha would be appreciated. ;)

-- 
MPlayer RPMs maintainer: http://greysector.rangers.eu.org/mplayer.html
"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"