[MPlayer-dev-eng] [PATCH] Check if path is too long

[KO Myung-Hun]

Reimar Döffinger wrote:
> 
> 
> On 10.06.2013, at 16:59, KO Myung-Hun <komh78 at gmail.com> wrote:
> 
>>
>>
>> KO Myung-Hun wrote:
>>> Updated..
>>>
>>>
>>>
>>
>> I'll commit this tomorrow if no more comments...
> 
> I think it's quite fine in principle, but some minor nits:
> I think it might be nicer to just put it all into a single header.

Agree. How about osdep/osdep.h ?

> Next, I think you should be using macros with parameters, to avoid replacing e.g. variables. Though I admit this is a bit problematic since it means the replacement won't work if "open" was assigned to a function pointer.
> On the other hand it might allow implementing both open() variants without having to resort to varargs, which has different behaviour from the non-varargs variant in the specification (using varargs means that e.g. a float value would be passed as such instead of being converted to the specified mode_t type).
> I.e. something like
> #define open(a, b) mplayer_open(a, b, 0)

I'm sorry I don't understand what you are saying.

Do you mean that I should define two types of open() macros multiply,
like this ?
#define open(a, b) mplayer_open(a, b, 0)
#define open(a, b, c) mplayer_open(a, b, c)

However, this is not possible. hmm...

And open group specification use varargs for open() declaration.

    http://pubs.opengroup.org/onlinepubs/009695399/functions/open.html

BTW, how about using __VA_ARGS__ ?

> Which leads me to the last point: you seem to implement an open variant with 4 arguments, but there is only the 2 and 3 argument one (and only the 3 argument one seems to be actually used in MPlayer).

Yes, I also know that the maximum number of arguments of open() is three
in the above specification. And 4 arguments open() variant is used by
OS/2 libc(EMX and kLIBC).

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.7.2
Under OS/2 Warp 4 for Korean with FixPak #15
In VirtualBox v4.1.24 on Intel Core i7-3615QM 2.30GHz with 8GB RAM

Korean OS/2 User Community : http://www.ecomstation.co.kr