[MPlayer-dev-eng] [PATCH] CDDB support for OS/2

[Diego Biurrun]

On Wed, Feb 24, 2010 at 11:02:50PM +0900, KO Myung-Hun wrote:
> Reimar Döffinger wrote:
>> On Sun, Feb 21, 2010 at 12:08:15AM +0900, KO Myung-Hun wrote:
>>> Reimar Döffinger wrote:
>>>> On Fri, Feb 19, 2010 at 10:28:47PM +0900, KO Myung-Hun wrote:
>>>>>>> @@ -340,7 +423,7 @@
>>>>>>>   	sprintf( file_name, "%s%08lx", cddb_data->cache_dir, cddb_data->disc_id);
>>>>>>>
>>>>>>>   	file_fd = open(file_name, O_RDONLY
>>>>>>> -#if defined(__MINGW32__) || defined(__CYGWIN__)
>>>>>>> +#if defined(__MINGW32__) || defined(__CYGWIN__) || defined(__OS2__)
>>>>>>>   	| O_BINARY
>>>>>>>   #endif
>>>>>>
>>>>>> This looks like a candidate for a configure check.
>>>>>
>>>>> So do I. I'll try later such as HAVE_DOS_BINARY ? Hmm.. poor nameing sense.
>>>>
>>>> #ifndef O_BINARY
>>>> #define O_BINARY 0
>>>> #endif
>>>> for example.
>>>> Don't see why this should depend on the OS instead of whether O_BINARY exists.
>>>
>>> I also know this approach. But the reason why I hesitate to use this
>>> is that BeOS also this flag but I'm not sure it has the same meaning
>>> as Win32 and OS/2.
>>>
>>> Are you sure ?
>>
>> All information I can find says that BeOS only has O_BINARY for compatibility,
>> it has no effect at all.
>> So it sounds safe to me, at least good enough to try IMHO.
>
> Ok. I tried.

OK, this shows how you want to use it in your cddb port.

Maybe you can use it to refactor the existing OS/2 header mess first...

> --- stream/stream_cddb.c	(revision 30696)
> +++ stream/stream_cddb.c	(working copy)
> @@ -68,12 +68,18 @@
>  #include "network.h"
>  #include "libavutil/common.h"
>  
> +#ifndef O_BINARY
> +#define O_BINARY 0
> +#endif
> +
>  #define DEFAULT_FREEDB_SERVER "freedb.freedb.org"
>  #define DEFAULT_CACHE_DIR     "/.cddb/"
>  
> @@ -349,11 +427,7 @@
>  
> -    file_fd = open(file_name, O_RDONLY
> -#if defined(__MINGW32__) || defined(__CYGWIN__)
> -    | O_BINARY
> -#endif
> -   );
> +    file_fd = open(file_name, O_RDONLY | O_BINARY);
>      if (file_fd < 0) {

This is unrelated and I think Reimar already OKed it.  Please commit
right away.

Diego