[Mesa-dev] [PATCH 12/18] swr: [rasterizer common] OSX cleanups

[Emil Velikov]

On 17 May 2016 at 17:56, Rowley, Timothy O <timothy.o.rowley at intel.com> wrote:
>
>> On May 17, 2016, at 5:56 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Afaics, things were designed around the premise that one should be
>> Windows/MSVC (?) compatible, correct ?
>>
>> In many places you're using the native types and hopefully soon you
>> can 'flip' the compat. to be used for windows (non-posix) systems.
>
> The primary development of the rasterizer actually happens on Windows/MSVC; while portability was always desired, it sometimes came with a windows flavor.  It’s moving in the direction of using standard C++ language constructs with hopefully minimal platform dependencies.

Looking closer at this series, it feels that there's little intention
of getting rid of it, as more "Windows compatibility" macros are
added.
Can we please keep things clearer in the open-source part ? If your
colleagues people prefer to use the Windows named types/functions,
just throw it in a header that is private to them ?

Pretty please ?

Note this is not an objection against this series, but a request that
things slowly get fixed. Or at least "don't get worse", sort of speak.

>>
>> On 17 May 2016 at 02:10, Tim Rowley <timothy.o.rowley at intel.com> wrote:
>>> ---
>>> src/gallium/drivers/swr/rasterizer/common/os.h              | 9 ++++++++-
>>> src/gallium/drivers/swr/rasterizer/common/rdtsc_buckets.cpp | 2 +-
>>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/swr/rasterizer/common/os.h b/src/gallium/drivers/swr/rasterizer/common/os.h
>>> index 94bb567..51e6c5a 100644
>>> --- a/src/gallium/drivers/swr/rasterizer/common/os.h
>>> +++ b/src/gallium/drivers/swr/rasterizer/common/os.h
>>> @@ -68,7 +68,7 @@ static inline void AlignedFree(void* p)
>>> #define _mm_popcount_sizeT _mm_popcnt_u32
>>> #endif
>>>
>>> -#elif defined(FORCE_LINUX) || defined(__linux__) || defined(__gnu_linux__)
>>> +#elif defined(__APPLE__) || defined(FORCE_LINUX) || defined(__linux__) || defined(__gnu_linux__)
>>>
>>> #define SWR_API
>>>
>>> @@ -80,6 +80,7 @@ static inline void AlignedFree(void* p)
>>> #include <unistd.h>
>>> #include <sys/stat.h>
>>> #include <stdio.h>
>>> +#include <limits.h>
>>>
>>> typedef void            VOID;
>>> typedef void*           LPVOID;
>>> @@ -95,6 +96,8 @@ typedef unsigned int    DWORD;
>>> #undef TRUE
>>> #define TRUE 1
>>>
>>> +#define MAX_PATH PATH_MAX
>>> +
>>> #define OSALIGN(RWORD, WIDTH) RWORD __attribute__((aligned(WIDTH)))
>>> #define THREAD __thread
>>> #ifndef INLINE
>>> @@ -194,6 +197,10 @@ void AlignedFree(void* p)
>>>     free(p);
>>> }
>>>
>>> +#define _countof(a) (sizeof(a)/sizeof(*(a)))
>>> +
>> Looks unused ?
>
> This is used by a portion of code not exposed in the open source view.
>
Arr... don't think this was mentioned when swr was initially proposed
for inclusion. For example there's very little 'dead code' brought by
the VMWare people, so it would be nice if the same happens here as
well.

>>> +#define sprintf_s sprintf
>>> +#define strcpy_s(dst,size,src) strncpy(dst,src,size)
>> As/if you go with my suggestion above, you can reuse the str functions
>> in auxiliary/util/u_string.h, which should handle windows nicely.
>> Although honestly I hope they fixed their api to be threadsafe, as
>> defined by ISO C.
>
> If compilers supported C11, we could just use the _s variants without portability macros.  Unfortunately compilers seem to be lagging on this front.
>
Which compilers - gcc and clang should be fine with most/all things ?
Since you guys already toggle -std=c++11 capable, what is stopping us
from doing -std=c11 for the C sources ?

Thanks
Emil