[vlc-devel] [vlc-commits] compat: replace aligned_alloc() rather than posix_memalign()
Steve Lhomme
robux4 at gmail.com
Tue Jun 20 12:44:23 CEST 2017
On Tue, Jun 20, 2017 at 12:34 PM, <remi at remlab.net> wrote:
> The OS kernel allocates memory at page level. Depending where the line is
> drawn between OS and the C & C++ runtimes, there may be some higher level
Yes, the line where to draw the limit can be thin. But here you rely
on functions that do not match anything in the Windows OS and that
exist in no Windows toolchain. So either we decide to drop Windows
support because all we want is pure C11 code. Or we do things
differently.
> function provided by the OS.
>
> Still, fact is that standard allocation functions and free() are toolchain
> responsibility to provide on top of the OS specifics (Windows has its own
> heap management...). Another fact is that aligned_alloc() does not require
> any OS functionality that malloc() wouldn't also. And last fact is that VLC
> is written in ISO C and C++ which require all the functions above.
>
>
> Le 20 juin 2017 13:18:22 GMT+03:00, Steve Lhomme <robux4 at gmail.com> a écrit
> :
>>
>> On Tue, Jun 20, 2017 at 12:14 PM, <remi at remlab.net> wrote:
>>>
>>> It has everything to do with the toolchain. The toolchain provides the
>>> hosted ISO C library.
>>
>>
>> The toolchain doesn't do the allocation, the OS does. And it also
>> imposes how this memory needs to be free'd.
>>
>> But you can argue endlessly on what part is broken, the fact is the
>> core is currently unusable on Windows due to your patches. And your
>> best answer is "I don't care".
>>
>>> Le 20 juin 2017 13:05:49 GMT+03:00, Steve Lhomme <robux4 at gmail.com> a
>>> écrit
>>> :
>>>>
>>>>
>>>> On Tue, Jun 20, 2017 at 11:53 AM, <remi at remlab.net> wrote:
>>>>>
>>>>>
>>>>> Repeating myself but this patch does not change the fallback logic,
>>>>> so
>>>>> it
>>>>> can't break anything.
>>>>>
>>>>> And to paraphrase other devs "I don't care" about non-ISO toolchains.
>>>>
>>>>
>>>>
>>>> The fact on Windows the aligned malloc needs to be freed with an
>>>> aligned version of free() has nothing to do with the toolchain.
>>>>
>>>>>
>>>>> Le 20 juin 2017 12:40:41 GMT+03:00, Steve Lhomme <robux4 at gmail.com> a
>>>>> écrit
>>>>> :
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 20, 2017 at 11:35 AM, Steve Lhomme <robux4 at gmail.com>
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Jun 20, 2017 at 11:28 AM, <remi at remlab.net> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The goal is to use aligned_alloc() instead of posix_memalign()
>>>>>>>> since
>>>>>>>> the
>>>>>>>> earlier is required by ISO C and C++ (and slightly simpler to
>>>>>>>> use).
>>>>>>>>
>>>>>>>> This patch only changes the behaviour on supported non-POSIX
>>>>>>>> toolchains
>>>>>>>> by
>>>>>>>> removing the need for POSIX emulation code. If the toolchain is
>>>>>>>> broken,
>>>>>>>> then
>>>>>>>> the fallback logic is unchanged: posix_memalign(), else
>>>>>>>> memalign()
>>>>>>>> else
>>>>>>>> screw-you.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> There is no posix_memalign() or memalign() on Windows. So right
>>>>>>> now
>>>>>>> VLC and libvlc cannot be used at all.
>>>>>>>
>>>>>>> Proper support requires using __mingw_aligned_malloc() or
>>>>>>> _aligned_malloc(). But that means using __mingw_aligned_free() and
>>>>>>> _aligned_free() as well.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> For the record, calling free() instead of these aligned free
>>>>>> functions
>>>>>> results in a crash.
>>>>>>
>>>>>>> Next time you may break the core, please submit patches for review
>>>>>>> first.
>>>>>>>
>>>>>>>> Le 20 juin 2017 12:07:38 GMT+03:00, Steve Lhomme
>>>>>>>> <robux4 at gmail.com>
>>>>>>>> a
>>>>>>>> écrit
>>>>>>>> :
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Sat, Jun 17, 2017 at 2:35 PM, Rémi Denis-Courmont
>>>>>>>>> <git at videolan.org>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> |
>>>>>>>>>> Sat
>>>>>>>>>> Jun
>>>>>>>>>> 17 15:27:16 2017 +0300|
>>>>>>>>>> [34cd965645cb0246f3d74515bbd5e55367f7d884]
>>>>>>>>>> |
>>>>>>>>>> committer: Rémi Denis-Courmont
>>>>>>>>>>
>>>>>>>>>> compat: replace aligned_alloc() rather than posix_memalign()
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=34cd965645cb0246f3d74515bbd5e55367f7d884
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> compat/{posix_memalign.c => aligned_alloc.c} | 62
>>>>>>>>>> ++++++++++++++--------------
>>>>>>>>>> configure.ac | 4 +-
>>>>>>>>>> include/vlc_common.h | 8 +---
>>>>>>>>>> include/vlc_fixups.h | 6 +--
>>>>>>>>>> 4 files changed, 36 insertions(+), 44 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/compat/posix_memalign.c b/compat/aligned_alloc.c
>>>>>>>>>> similarity index 55%
>>>>>>>>>> rename from compat/posix_memalign.c
>>>>>>>>>> rename to compat/aligned_alloc.c
>>>>>>>>>> index 3c6ffdfe28..9a61c0010f 100644
>>>>>>>>>> --- a/compat/posix_memalign.c
>>>>>>>>>> +++ b/compat/aligned_alloc.c
>>>>>>>>>> @@ -1,7 +1,7 @@
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> /*****************************************************************************
>>>>>>>>>> - * posix_memalign.c: POSIX posix_memalign() replacement
>>>>>>>>>> + * aligned_alloc.c: C11 aligned_alloc() replacement
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *****************************************************************************
>>>>>>>>>> - * Copyright © 2012 Rémi Denis-Courmont
>>>>>>>>>> + * Copyright © 2012, 2017 Rémi Denis-Courmont
>>>>>>>>>> *
>>>>>>>>>> * 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
>>>>>>>>>> @@ -22,46 +22,44 @@
>>>>>>>>>> # include <config.h>
>>>>>>>>>> #endif
>>>>>>>>>>
>>>>>>>>>> +#include <assert.h>
>>>>>>>>>> #include <stdlib.h>
>>>>>>>>>> #include <errno.h>
>>>>>>>>>> +#if !defined (HAVE_POSIX_MEMALIGN) && !defined (_WIN32) &&
>>>>>>>>>> !defined
>>>>>>>>>> (__APPLE__)
>>>>>>>>>> +# include <malloc.h>
>>>>>>>>>> +#endif
>>>>>>>>>>
>>>>>>>>>> -static int check_align (size_t align)
>>>>>>>>>> +void *aligned_alloc(size_t align, size_t size)
>>>>>>>>>> {
>>>>>>>>>> - for (size_t i = sizeof (void *); i != 0; i *= 2)
>>>>>>>>>> - if (align == i)
>>>>>>>>>> - return 0;
>>>>>>>>>> - return EINVAL;
>>>>>>>>>> -}
>>>>>>>>>> -
>>>>>>>>>> -#if !defined (_WIN32) && !defined (__APPLE__)
>>>>>>>>>> -#include <malloc.h>
>>>>>>>>>> + /* align must be a power of 2 */
>>>>>>>>>> + /* size must be a multiple of align */
>>>>>>>>>> + if ((align & (align - 1)) || (size & (align - 1)))
>>>>>>>>>> + {
>>>>>>>>>> + errno = EINVAL;
>>>>>>>>>> + return NULL;
>>>>>>>>>> + }
>>>>>>>>>>
>>>>>>>>>> -int posix_memalign (void **ptr, size_t align, size_t size)
>>>>>>>>>> -{
>>>>>>>>>> - if (check_align (align))
>>>>>>>>>> - return EINVAL;
>>>>>>>>>> +#ifdef HAVE_POSIX_MEMALIGN
>>>>>>>>>> + if (align < sizeof (void *)) /* POSIX does not allow
>>>>>>>>>> small
>>>>>>>>>> alignment */
>>>>>>>>>> + align = sizeof (void *);
>>>>>>>>>>
>>>>>>>>>> - int saved_errno = errno;
>>>>>>>>>> - void *p = memalign (align, size);
>>>>>>>>>> - if (p == NULL)
>>>>>>>>>> + void *ptr;
>>>>>>>>>> + int err = posix_memalign(&ptr, align, size);
>>>>>>>>>> + if (err)
>>>>>>>>>> {
>>>>>>>>>> - errno = saved_errno;
>>>>>>>>>> - return ENOMEM;
>>>>>>>>>> + errno = err;
>>>>>>>>>> + ptr = NULL;
>>>>>>>>>> }
>>>>>>>>>> + return ptr;
>>>>>>>>>>
>>>>>>>>>> - *ptr = p;
>>>>>>>>>> - return 0;
>>>>>>>>>> -}
>>>>>>>>>> +#elif !defined (_WIN32) && !defined (__APPLE__)
>>>>>>>>>>
>>>>>>>>>> -#else
>>>>>>>>>> -
>>>>>>>>>> -int posix_memalign (void **ptr, size_t align, size_t size)
>>>>>>>>>> -{
>>>>>>>>>> - if (check_align (align))
>>>>>>>>>> - return EINVAL;
>>>>>>>>>> + return memalign(align, size);
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Is the goal that this code is never called for Windows and Apple
>>>>>>>>> OSes
>>>>>>>>> ? Because it worked and this function always return NULL.
>>>>>>>>>
>>>>>>>>>> - *ptr = NULL;
>>>>>>>>>> - return size ? ENOMEM : 0;
>>>>>>>>>> -}
>>>>>>>>>> +#else
>>>>>>>>>>
>>>>>>>>>> + if (size > 0)
>>>>>>>>>> + errno = ENOMEM;
>>>>>>>>>> + return NULL;
>>>>>>>>>> #endif
>>>>>>>>>> +}
>>>>>>>>>> diff --git a/configure.ac b/configure.ac
>>>>>>>>>> index b1883a18b7..67fc7deb1a 100644
>>>>>>>>>> --- a/configure.ac
>>>>>>>>>> +++ b/configure.ac
>>>>>>>>>> @@ -597,8 +597,8 @@ dnl Check for system libs needed
>>>>>>>>>> need_libc=false
>>>>>>>>>>
>>>>>>>>>> dnl Check for usual libc functions
>>>>>>>>>> -AC_CHECK_FUNCS([daemon fcntl flock fstatvfs fork getenv
>>>>>>>>>> getpwuid_r
>>>>>>>>>> isatty lstat memalign mkostemp mmap open_memstream openat pread
>>>>>>>>>> posix_fadvise posix_madvise setlocale stricmp strnicmp strptime
>>>>>>>>>> tdestroy
>>>>>>>>>> uselocale])
>>>>>>>>>> -AC_REPLACE_FUNCS([atof atoll dirfd fdopendir ffsll flockfile
>>>>>>>>>> fsync
>>>>>>>>>> getdelim getpid lldiv memrchr nrand48 poll posix_memalign
>>>>>>>>>> recvmsg
>>>>>>>>>> rewind
>>>>>>>>>> sendmsg setenv strcasecmp strcasestr strdup strlcpy strndup
>>>>>>>>>> strnlen
>>>>>>>>>> strnstr
>>>>>>>>>> strsep strtof strtok_r strtoll swab tfind timegm timespec_get
>>>>>>>>>> strverscmp
>>>>>>>>>> pathconf])
>>>>>>>>>> +AC_CHECK_FUNCS([daemon fcntl flock fstatvfs fork getenv
>>>>>>>>>> getpwuid_r
>>>>>>>>>> isatty lstat memalign mkostemp mmap open_memstream openat pread
>>>>>>>>>> posix_fadvise posix_madvise posix_memalign setlocale stricmp
>>>>>>>>>> strnicmp
>>>>>>>>>> strptime tdestroy uselocale])
>>>>>>>>>> +AC_REPLACE_FUNCS([aligned_alloc atof atoll dirfd fdopendir
>>>>>>>>>> ffsll
>>>>>>>>>> flockfile fsync getdelim getpid lldiv memrchr nrand48 poll
>>>>>>>>>> recvmsg
>>>>>>>>>> rewind
>>>>>>>>>> sendmsg setenv strcasecmp strcasestr strdup strlcpy strndup
>>>>>>>>>> strnlen
>>>>>>>>>> strnstr
>>>>>>>>>> strsep strtof strtok_r strtoll swab tfind timegm timespec_get
>>>>>>>>>> strverscmp
>>>>>>>>>> pathconf])
>>>>>>>>>> AC_REPLACE_FUNCS([gettimeofday])
>>>>>>>>>> AC_CHECK_FUNC(fdatasync,,
>>>>>>>>>> [AC_DEFINE(fdatasync, fsync, [Alias fdatasync() to fsync()
>>>>>>>>>> if
>>>>>>>>>> missing.])
>>>>>>>>>> diff --git a/include/vlc_common.h b/include/vlc_common.h
>>>>>>>>>> index e1cf885379..0bfb1b9b04 100644
>>>>>>>>>> --- a/include/vlc_common.h
>>>>>>>>>> +++ b/include/vlc_common.h
>>>>>>>>>> @@ -854,13 +854,7 @@ VLC_API bool vlc_ureduce( unsigned *,
>>>>>>>>>> unsigned
>>>>>>>>>> *,
>>>>>>>>>> uint64_t, uint64_t, uint64_t )
>>>>>>>>>> # define vlc_memalign(align, size) (_aligned_malloc(size,
>>>>>>>>>> align))
>>>>>>>>>> # define vlc_free(base) (_aligned_free(base))
>>>>>>>>>> #else
>>>>>>>>>> -static inline void *vlc_memalign(size_t align, size_t size)
>>>>>>>>>> -{
>>>>>>>>>> - void *base;
>>>>>>>>>> - if (unlikely(posix_memalign(&base, align, size)))
>>>>>>>>>> - base = NULL;
>>>>>>>>>> - return base;
>>>>>>>>>> -}
>>>>>>>>>> +# define vlc_memalign(align, size) aligned_alloc(align, size)
>>>>>>>>>> # define vlc_free(base) free(base)
>>>>>>>>>> #endif
>>>>>>>>>>
>>>>>>>>>> diff --git a/include/vlc_fixups.h b/include/vlc_fixups.h
>>>>>>>>>> index 8bfb76888f..44b8426f2c 100644
>>>>>>>>>> --- a/include/vlc_fixups.h
>>>>>>>>>> +++ b/include/vlc_fixups.h
>>>>>>>>>> @@ -88,7 +88,7 @@ typedef struct
>>>>>>>>>> # include <stdio.h> /* FILE */
>>>>>>>>>> #endif
>>>>>>>>>>
>>>>>>>>>> -#if !defined (HAVE_POSIX_MEMALIGN) || \
>>>>>>>>>> +#if !defined (HAVE_ALIGNED_ALLOC) || \
>>>>>>>>>> !defined (HAVE_MEMRCHR) || \
>>>>>>>>>> !defined (HAVE_STRLCPY) || \
>>>>>>>>>> !defined (HAVE_STRNDUP) || \
>>>>>>>>>> @@ -302,8 +302,8 @@ int setenv (const char *, const char *,
>>>>>>>>>> int);
>>>>>>>>>> int unsetenv (const char *);
>>>>>>>>>> #endif
>>>>>>>>>>
>>>>>>>>>> -#ifndef HAVE_POSIX_MEMALIGN
>>>>>>>>>> -int posix_memalign (void **, size_t, size_t);
>>>>>>>>>> +#ifndef HAVE_ALIGNED_ALLOC
>>>>>>>>>> +void *aligned_alloc(size_t, size_t);
>>>>>>>>>> #endif
>>>>>>>>>>
>>>>>>>>>> #if defined(__native_client__) && defined(__cplusplus)
>>>>>>>>>>
>>>>>>>>>> ________________________________
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> vlc-commits mailing list
>>>>>>>>>> vlc-commits at videolan.org
>>>>>>>>>> https://mailman.videolan.org/listinfo/vlc-commits
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ________________________________
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>
>>>
>>>
>>> --
>>> 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
>
>
> --
> 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
More information about the vlc-devel
mailing list