Re: [Tarantool-patches] [PATCH luajit 08/19] Windows: Add UWP support, part 1.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Maxim!
Thanks for the review!
Fixed your comments inline.

On 15.08.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few comments below.
> On Wed, Aug 09, 2023 at 06:35:57PM +0300, Sergey Kaplun via Tarantool-patches wrote:
> > From: Mike Pall <mike>
> > 
> > Contributed by Ben Pye.
> > 
> > (cherry-picked from commit c3c54ce1aef782823936808a75460e6b53aada2c)
> > 
> > This patch adds partial support for the Universal Windows Platform [1]
> > in LuaJIT.
> > This includes:
> > * `LJ_TARGET_UWP` is introduced to mark that target is Universal Windows
> Typo: s/is Unviersal/is the Universal/

Fixed.

> >   Platform.
> > * `LJ_WIN_VALLOC()` macro is introduced to use instead of
> Typo: s/to use/to be used/

Fixed.

> >   `VirtualAlloc()` [2] (`VirtualAllocFromApp()` [3] for UWP)
> > * `LJ_WIN_VPROTECT()` macro is introduced to use instead of
> Typo: s/to use/to be used/

Fixed.

> >   `VirtualProtect()` [4] (`VirtualProtectFromApp()` [5] for UWP)
> > * `LJ_WIN_LOADLIBA()` macro is introduced to use instead of
> Typo: s/to use/to be used/

Fixed.

> >   `LoadLibraryExA()` [6] (custom implementation using
> >   `LoadPackagedLibrary()` [7] for UWP).
> > 
> > Note that the following features are not implemented for UWP:
> > * `io.popen()`.
> > * LuaJIT profiler's (`jit.p`) timer for Windows has not very high
> >   resolution since `timeBeginPeriod()` [8] and `timeEndPeriod()` [9] are
> Typo: s/not very high/a low/

Fixed.

> >   not used, because the <winmm.dll> library isn't loaded.
> > 
> > [1]: https://learn.microsoft.com/en-us/windows/uwp/get-started/universal-application-platform-guide
> > [2]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualalloc
> > [3]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualallocfromapp
> > [4]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualprotect
> > [5]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualprotectfromapp
> > [6]: https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa
> > [7]: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-loadpackagedlibrary
> > [8]: https://learn.microsoft.com/en-us/windows/win32/api/timeapi/nf-timeapi-timebeginperiod
> > [9]: https://learn.microsoft.com/en-us/windows/win32/api/timeapi/nf-timeapi-timeendperiod
> > 
> > Sergey Kaplun:
> > * added the description for the feature
> > 
> > Part of tarantool/tarantool#8825
> > ---

<snipped>

> > +#if LJ_TARGET_UWP
> > +  case H_(a40f0bcb,a40f0bcb): b = 1; break;  /* uwp */
> >  #endif
> It is not obvious what happens here and it is not mentioned in the commit message.
> Please add a description of this change too.

Added.

> >    case H_(3af93066,1f001464): b = 1; break;  /* le/be */

The new commit message is the following:

| Windows: Add UWP support, part 1.
|
| Contributed by Ben Pye.
|
| (cherry-picked from commit c3c54ce1aef782823936808a75460e6b53aada2c)
|
| This patch adds partial support for the Universal Windows Platform [1]
| in LuaJIT.
| This includes:
| * `LJ_TARGET_UWP` is introduced to mark that target is the Universal
|   Windows Platform.
| * `LJ_WIN_VALLOC()` macro is introduced to be used instead of
|   `VirtualAlloc()` [2] (`VirtualAllocFromApp()` [3] for UWP)
| * `LJ_WIN_VPROTECT()` macro is introduced to be used instead of
|   `VirtualProtect()` [4] (`VirtualProtectFromApp()` [5] for UWP)
| * `LJ_WIN_LOADLIBA()` macro is introduced to be used instead of
|   `LoadLibraryExA()` [6] (custom implementation using
|   `LoadPackagedLibrary()` [7] for UWP).
| * Now `ffi.abi()` also provides information about "uwp" parameter for
|   target ABI.
|
| Note that the following features are not implemented for UWP:
| * `io.popen()`.
| * LuaJIT profiler's (`jit.p`) timer for Windows has a low resolution
|   since `timeBeginPeriod()` [8] and `timeEndPeriod()` [9] are not used,
|   because the <winmm.dll> library isn't loaded.
|
| [1]: https://learn.microsoft.com/en-us/windows/uwp/get-started/universal-application-platform-guide
| [2]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualalloc
| [3]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualallocfromapp
| [4]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualprotect
| [5]: https://learn.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualprotectfromapp
| [6]: https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa
| [7]: https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-loadpackagedlibrary
| [8]: https://learn.microsoft.com/en-us/windows/win32/api/timeapi/nf-timeapi-timebeginperiod
| [9]: https://learn.microsoft.com/en-us/windows/win32/api/timeapi/nf-timeapi-timeendperiod
|
| Sergey Kaplun:
| * added the description for the feature
|
| Part of tarantool/tarantool#8825


<snipped>

> > 

-- 
Best regards,
Sergey Kaplun