[vlc-devel] [PATCH 1/2] include: add vlc_search.h for differences between POSIX search.h and other OS
Steve Lhomme
robux4 at ycbcr.xyz
Mon Jul 15 12:11:01 CEST 2019
On 2019-07-15 12:03, Steve Lhomme wrote:
> On 2019-07-15 11:53, Steve Lhomme wrote:
>> On 2019-07-15 9:37, Rémi Denis-Courmont wrote:
>>> Hi,
>>>
>>> A macro is possible, probably:
>>>
>>> #include<search.h>
>>> #define lfind(a,b,c,d,e) \
>>> lfind(a,b&(unsigned){ *(c) },d,e)
>>
>> That works for win32 but not for win64 where 'size_t' and 'unsigned'
>> don't have the same size, as verified with this:
>
> Or maybe I misread the patch and it doesn't do a cast, just creates a
> local variable ? In that case it should work (it's missing a comma after
> 'b').
>
> Still not sure we're supposed to fill a value in c after the call. But
> it's not used in the code using lfind and the implementation in compat/
> doesn't fill it either.
Also we can't put this in vlc_fixup because as soon as you include
search.h its definition is turned into this macro. For now we could just
put the macro in dtv/access.c or we need a new include. (Which we can
always forget to include where lfind() is used, but there will be a
warning on Windows)
>> static_assert(sizeof(size_t) == sizeof(unsigned), "cannot cast for
>> lfind");
>>
>>> Le 15 juillet 2019 10:07:26 GMT+03:00, Steve Lhomme
>>> <robux4 at ycbcr.xyz> a écrit :
>>>> Hi
>>>>
>>>> On 2019-07-12 16:35, Hugo Beauzée-Luyssen wrote:
>>>>> On Fri, Jul 12, 2019, at 3:51 PM, Steve Lhomme wrote:
>>>>>> ---
>>>>>> include/vlc_search.h | 52
>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>>> src/Makefile.am | 3 ++-
>>>>>> 2 files changed, 54 insertions(+), 1 deletion(-)
>>>>>> create mode 100644 include/vlc_search.h
>>>>>>
>>>>>> diff --git a/include/vlc_search.h b/include/vlc_search.h
>>>>>> new file mode 100644
>>>>>> index 0000000000..6c87fa990d
>>>>>> --- /dev/null
>>>>>> +++ b/include/vlc_search.h
>>>>>> @@ -0,0 +1,52 @@
>>>>>>
>>>> +/*****************************************************************************
>>>>
>>>>>> + * vlc_search.h: portability fixups included from config.h
>>>>>> +
>>>>>>
>>>> *****************************************************************************
>>>>
>>>>>> + * Copyright © 2019 the VideoLAN project
>>>>>> + *
>>>>>> + * This program is free software; you can redistribute it and/or
>>>>>> modify it
>>>>>> + * under the terms of the GNU Lesser General Public License as
>>>>>> published by
>>>>>> + * the Free Software Foundation; either version 2.1 of the License,
>>>> or
>>>>>> + * (at your option) any later version.
>>>>>> + *
>>>>>> + * This program is distributed in the hope that it will be useful,
>>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>>>> + * GNU Lesser General Public License for more details.
>>>>>> + *
>>>>>> + * You should have received a copy of the GNU Lesser General Public
>>>>>> License
>>>>>> + * along with this program; if not, write to the Free Software
>>>>>> Foundation,
>>>>>> + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301,
>>>> USA.
>>>>>> +
>>>>>>
>>>> *****************************************************************************/
>>>>
>>>>>> +
>>>>>> +/**
>>>>>> + * \file
>>>>>> + * This file is a collection of portability fixes
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef LIBVLC_SEARCH_H
>>>>>> +# define LIBVLC_SEARCH_H 1
>>>>>> +
>>>>>> +#ifdef __cplusplus
>>>>>> +extern "C" {
>>>>>> +#endif
>>>>>> +
>>>>>> +#ifdef _WIN32
>>>>>> +/* the Win32 prorotype of lfind() expects an unsigned int for
>>>> 'nelp' */
>>>>>
>>>>> Typo (proTotype)
>>>>>
>>>>>> +static inline void *vlc_lfind(const void *key, const void *base,
>>>>>> size_t *nelp,
>>>>>> + size_t size, int(*cmp)(const void *,
>>>>>> const void *))
>>>>>> +{
>>>>>> + unsigned int n;
>>>>>> + void *res = lfind (key, base, &n, size, cmp);
>>>>>> + *nelp = n;
>>>>>> + return res;
>>>>>
>>>>> I must say I don't understand what this wrapper does that wasn't
>>>> already done by calling the function with an uint32_t* instead of a
>>>> size_t*
>>>>> If what you want is to fix the warning on the callsite, you can
>>>> probably get away with a macro that casts the 3rd pointer no?
>>>>
>>>> Are "size_t" and "unsigned int" guaranteed to be the same on all
>>>> Windows
>>>> compiler ? I think on 64 bits platforms it's not the case.
>>>>
>>>> So we would need a wrapper with the proper size_t variable. To do that
>>>> I
>>>> don't think you can do it as a pure macro because you also need to set
>>>> the value of the size_t nelp to the value of the unsigned int and then
>>>> pass that unsigned int as a pointer. (not sure the value on return
>>>> needs
>>>> to be set back on the size_t, the lfind documentation is not clear)
>>>>
>>>> So if it's not a macro, it should be an inline. But to do an inline you
>>>>
>>>> need to include <search.h> so it knows about lfind(). This cannot be
>>>> added in vlc_fixups.h
>>>>
>>>>> And in any case case:
>>>>> - Does this need to be in a different file?
>>>>
>>>> IMO, yes.
>>>>
>>>>> - It could be enable only when _WIN64 is defined
>>>>
>>>> OK but it doesn't hurt to have the same code between 32 and 64 bits
>>>> Windows.
>>>>
>>>>>> +}
>>>>>> +#else /* !_WIN32 */
>>>>>> +#define vlc_lfind(key, base, nelp, width, compar) \
>>>>>> + lfind(key, base, nelp, width, compar)
>>>>>> +#endif /* !_WIN32 */
>>>>>> +
>>>>>> +#ifdef __cplusplus
>>>>>> +} /* extern "C" */
>>>>>> +#endif
>>>>>> +
>>>>>> +#endif /* !LIBVLC_SEARCH_H */
>>>>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>>>>> index 006ece5f93..25f9a4fc8b 100644
>>>>>> --- a/src/Makefile.am
>>>>>> +++ b/src/Makefile.am
>>>>>> @@ -88,6 +88,7 @@ pluginsinclude_HEADERS = \
>>>>>> ../include/vlc_fingerprinter.h \
>>>>>> ../include/vlc_interrupt.h \
>>>>>> ../include/vlc_renderer_discovery.h \
>>>>>> + ../include/vlc_search.h \
>>>>>> ../include/vlc_sort.h \
>>>>>> ../include/vlc_sout.h \
>>>>>> ../include/vlc_spu.h \
>>>>>> @@ -591,7 +592,7 @@ test_mrl_helpers_SOURCES = test/mrl_helpers.c
>>>>>> test_arrays_SOURCES = test/arrays.c
>>>>>> test_vector_SOURCES = test/vector.c
>>>>>> test_shared_data_ptr_SOURCES = test/shared_data_ptr.cpp
>>>>>> -test_extensions_SOURCES = test/extensions.c
>>>>>> +test_extensions_SOURCES = test/extensions.c
>>>>>
>>>>> Apparently my editor didn't remove the whitespace, sorry about that.
>>>> Although this shouldn't be here :)
>>>>
>>>> ok
>>>>
>>>>>> test_playlist_SOURCES = playlist/test.c \
>>>>>> playlist/content.c \
>>>>>> playlist/control.c \
>>>>>
>>>>> Regards,
>>>>>
>>>>> --
>>>>> Hugo Beauzée-Luyssen
>>>>> hugo at beauzee.fr
>>>>> _______________________________________________
>>>>> vlc-devel mailing list
>>>>> To unsubscribe or modify your subscription options:
>>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>>>
>>>> _______________________________________________
>>>> vlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>>> --
>>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>>> excuser ma brièveté.
>>>
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list