[vlc-devel] [PATCH] file extension manager (fixed patch)

[ßєŋ]

> The core _is_ in C. In fact, those declarations should be in a private
> header inside src/ rather than include/
>

Ok. I thought the VLC_EXPORT() macro was there to make the difference
between private and public declaration.

>>> Using asprintf() (or QString) would be simpler and less likely to break
>>> if someone changes the code later.
>>>
>>
>> I don't know anything about Qstuff; never used and never wanted to.
>> asprintf is a convenient GNU extension.
>
> Nope. VLC does provide asprintf() on non-GNU systems.
>

asprintf it is.

Anyway my changes in GUI code was mere workaround. If the VLC team
intends to accept this patch; or at least see any interest in dynamically
managing file extensions (I think they are some); may be it should be discuss
somewhere and I be glad to give you my point of view and ideas. There is
a wiki for that I guess.


>> Totally useless in that context. I'm copying exclusively file
>> extension that are never longer than
>> a few chars. size_t looks like  a waste too me.
>
> On 32-bits system, it won't make a difference. On 64-bits, they will both
> spend on register anyway.
>

Can't size_t be more than 32bit on 32bit system on certain circumstance ?
I guess it can. Anyway C lake a specific type for index. It's really
architecture
dependant (some oldies preffer 16bit index, some only have unsigned
indexing ...)
What we can expect from a C compiler is that it handles properly int
since it is the
most used index type. From my point of view using size_t should be use when you
know the loop may be quiet large but int is the best average choice for
«well known» finite loops. Using size_t point out this code may loop over large
buffer (eg larger than int size like a mmaped file or something that
usually used size_t).
But it's your call but i'm pretty sure most loop in VLC code use int index.

Notice that the function we are talking about is not standard string
op. It is just copying
an extension which has already been measured and is less that 6 bytes
long. Some other
functions in my codes (the one parsing user provided extension list)
may take advantage
of using size_t but these functions use index only for parsing a
single extension
 and walking in the string is done by incrementing the pointer. You
remark points out a
possible overflow in the parser when measuring an extension. In
certain rare circumstance
it could have lead to an infinite loop (need a whole int-indexable
buffer full of alpha numeric char :) ).
I have fixed that thank to you.

>>> With the public API, we may have multiple instances in the same process.
>>> I am afraid this is not thread-safe.
>>>
>>
>> Process or thread ?
>
> We can have multiple instances in a single process, in the same or in
> different thread.
>

Multiple instances of the core ? I mean there is necessary a part of the code
that initialise things before the program forks into thread or process.
 If it is a process, I don't care. Each have to initialise its
extension pool by calling fileext_init()
or used an already initialised one that became two of them after the
fork. Only thread trigger
race condition so I have to be sure that fileext_Init() is called
before any thread that might call
fileext public function is created. I thought the part of the code I
have added my init call was
matching this condition. If not please tell me where I have to do put it.

Regards,
-- 
B.