waitingsong / node-win32-api

win32 api
MIT License
430 stars 55 forks source link

Why use Buffers instead of plain integers for handles? #16

Closed Venryx closed 2 years ago

Venryx commented 5 years ago

I notice that the user32 functions in this library, such as FindWindowExW return Buffers as the window handles.

However, I've seen other implementation of these functions in ffi just use simple integers -- for example, at bottom of this answer: https://stackoverflow.com/a/40733935/2441655

Question 1: Since simple integers are easier to log and pass around in Javascript, why does this library use Buffers instead?

There must be some advantage to it, but I'd like to know what it is so I can weigh whether to keep using this library or make a variant that just uses simple integers/js-numbers as handles.

==========

Oh also, how do you create a new "Buffer handle" if you have a handle in integer/js-number form?

I assumed one could just do:

var handle = [function that gets window handle, as simple integer/js-number].
var handleBuffer = Buffer.alloc(4)
handleBuffer.writeInt32LE(handle, 0);

However, whenever I pass that handleBuffer object to the ffi functions, it ignores them/acts like they're invalid. So I must be creating them wrong.

Question 2: Is there a straight-forward way of creating handle-buffers based on an integer-handle?

waitingsong commented 5 years ago

The tests got many issues when define hwnd as number( 'int' or 'uint32')

npm ERR! code ELIFECYCLE
npm ERR! errno 3221225477
npm ERR! Exit status 3221225477

And tests got instability during multiple times repeat.

Also from docs

2019-07-14_191851 2019-07-14_191818 2019-07-14_192212

so i think better that hwnd should be a pointer (Buffer) .

Venryx commented 4 years ago

It's interesting that you encountered errors when using int/uint32 for handles, since every node-ffi function I've used, with handles represented as ints, has worked fine for me.

Are you sure the errors aren't just because you haven't converted all the test code to correctly submit integer handles?

As for the Windows docs showing to use a pointer-type: I don't think that's an issue, because node-ffi apparently handles the conversion automatically.

Venryx commented 4 years ago

By the way, the main reason I dislike using Buffers is that it causes the whole program to instantly crash if the handle/pointer/buffer is "stringified" anywhere.

So for example, if you add a console.log or assert call somewhere to, say, make sure a handle is valid, you can end up shooting yourself in the foot when a minute later that line gets hit, and ends up causing your program to crash unexpectedly, potentially losing debug information/context.

Anyway, I figured I would share this workaround -- for others who would rather just use number-based handles:

import {User32} from 'win32-api';

let replacements = [
    // I thought the lib source-code specified the type as "HANDLE", but apparently it gets converted into "uint64*" at some point (perhaps in gen_api_opts)
    //{oldValue: "HANDLE", newValue: "int32"},
    {oldValue: "uint64*", newValue: "int32"},
];

for (let [name, definitionArray] of Object.entries(User32.apiDef)) {
    console.log(`Name(${name}) Returns(${definitionArray[0]}) Arguments(${definitionArray[1]})`);

    for (let replacement of replacements) {
        if (definitionArray[0] === replacement.oldValue) {
            definitionArray[0] = replacement.newValue;
        }
        for (let [index, argumentType] of definitionArray[1].entries()) {
            if (argumentType === replacement.oldValue) {
                definitionArray[1][index] = replacement.newValue;   
            }
        }
    }
}

const user32 = User32.load();

The fix above does what it looks like: replaces the definitions for all the API entry-points to send/receive number handles instead of Buffer-based ones.

Naturally, this means the value-types don't match up with the library's Typescript definitions. There are a few ways to resolve this:

1) Use Typescript type-assertions: let handle: number = user32.GetForegroundWindow() as any;

2) Add prototype extensions:

declare global {
    interface Buffer { Int(): number; }
    interface Number { Buf(): Buffer; }
}
Object.prototype["Int"] = function() {
    //if (this instanceof Buffer) return ref.address(this); // convert
    return this; // handle is already number -- just return
}
Object.prototype["Buf"] = function() {
    //if (typeof this == "number") return ...; // convert (don't know how)
    return this; // handle is already number (matching modified definition-array) -- just return
}

All it does is provide a succinct, chainable way to perform the type conversion/assertion.

Usage: let handle = user32.GetForegroundWindow().Int();

3) Add Typescript overloads for the modified functions:

// add Typescript overloads for the functions which were modified to send/receive number-handles instead of Buffer-handles
declare module "win32-api/dist/lib/user32/api" {
    export interface Win32Fns {
        GetWindowTextW(hWnd: number, lpString: Buffer, nMaxCount: number): number
    }
}
// now when you type "user32.GetWindowTextW", you'll see it has an overload accepting a number-handle

4) Swap Buffer with number or number | Buffer throughout the library: (what I currently use)

type RealBuffer = Buffer;
declare module "win32-api/node_modules/win32-def/dist/lib/win-model/common" {
    //type HWND = number; // doesn't work, unfortunately
    //type Buffer = number; // works, but then thinks all non-handle Buffer params are numbers
    type Buffer = number | RealBuffer; // works, but requires cast to number for returned handles
}
// now Typescript knows this returns either a number or a Buffer -- just narrow that to "number"
let handle = user32.GetForegroundWindow() as number;

Anyway, overall this "use number handles" modification is working fine for the functions I'm currently using, so it's "good enough" for now.

Though it would be ideal to eventually either: A) Found out how to stop Buffer-handle stringification from insta-crashing the program. B) Update the library to use number-based handles, circumventing the issue, and making for easier logging/storing. (another option is to apply these changes in a fork, however that takes more work, and splits the future development/maintenance effort)

waitingsong commented 4 years ago

I don't know why always crash on my pc. I'll try your workaround later (lack of time). Thanks for your advice.

waitingsong commented 4 years ago

The conversion path: HANDLE -> PVOID -> _WIN64_HOLDER -> "_WIN64_HOLDER" -> int64* or int32* (os platform relative)

Venryx commented 4 years ago

Any update on this?

I'd like to help contribute to a shared repo reconstructing the win32-api, but the issue with the Buffer handles insta-crashing the program when logged, makes me hesitant to use it.

My workaround above is okay, but if investing time into the lib's expansion, it would be nice to have the change to plain-number handles become part of the base library (so I don't need to copy my workaround code to each project that uses the lib).

I guess the main point is that your computer had crashes when using plain number handles. That's odd since it doesn't happen for me, so perhaps we'll need to do some Google searches to find what might be causing it. Hopefully it's just a configuration issue rather than something intrinsic to plain-number handles. (I'm guessing it is just a config issue somewhere)

waitingsong commented 4 years ago

@Venryx I'd re-installed fresh OS (win 8.1), and plan to try new handle type at this weekend . Could you tell me what 's the type of the handle on win64, int32 or int64?

Plan:

  1. combine win-def and win-api into one mono repo
  2. change handle type to number

thanks .

Venryx commented 4 years ago

Could you tell me what 's the type of the handle on win64, int32 or int64?

I've been using int32, and it's been working fine.

And this StackOverflow answer appears to confirm that int32 is compatible/sufficient (regardless of whether the computer is 32-bit or 64-bit): https://stackoverflow.com/a/29526711/2441655

Plan:

  1. combine win-def and win-api into one mono repo
  2. change handle type to number

Awesome! Always nice when group aims align so they don't have to split their efforts. :)

waitingsong commented 4 years ago

@ Venryx Plan.1 accomplished with v7

waitingsong commented 4 years ago

@Venryx Hi, what's the type of POINTER_32 you using, number (uint32) or Buffer (uint32*) ? and LONG_PTR is number | bigint or Buffer

That to say what's type should be used for foo_PTR number or Buffer ?

https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types#POINTER_32

eg: https://docs.microsoft.com/zh-cn/windows/win32/gdi/colorref

typedef DWORD COLORREF;
typedef DWORD* LPCOLORREF;

Can we use number for both COLORREF and LPCOLORREF?

Venryx commented 4 years ago

I haven't used the COLORREF or LPCOLORREF structs in any of my projects, so I can't answer for those specific cases. However, I do know that wherever window handles were involved, using number instead of Buffer worked fine.

My guess is that a simple number type would work for those other structs as well, since node-ffi apparently translates values to the correct types (ie. a pointer) internally. (assuming the js-type can fit the data, of course)

Of course, there are still cases where you'll need Buffer, for example when you call an API function that merely modifies an object referenced-by-Buffer; but those are the exception rather than the rule, and for the simpler pass-object or receive-object-as-return-value cases, replacing Buffer with number has worked for the instances I've tried.

So basically, I would suggest either: 1) "incrementally" changing Buffer usages to number, as tested by people using the function and confirmed working. (eg. I've tested a number of calls that use window-handles, and they all work fine with number params and return-types) 2) Try out a few other cases of Buffer converted to number, and if all of them seem to work, assume they all do (of the direct-transfer type), and convert them all at once.

Sorry I can't be of more help! Just passing on what I've noticed in my time using node-ffi for win-api calls.

waitingsong commented 4 years ago

@Venryx pls try next version win32-api@^9.0.0 type HANDLE changed to number | bigint | string (uint64/uint32) string for https://github.com/node-ffi-napi/node-ffi-napi#v8-and-64-bit-types

waitingsong commented 4 years ago

Of course, there are still cases where you'll need Buffer, for example when you call an API function that merely modifies an object referenced-by-Buffer

It seems to work passing Buffer address: Demo knl_GetMOduleHandleExW.ts due to

GetModuleHandleExW(dwFlags: M.DWORD, lpModuleName: M.LPCTSTR | null, phModule: M.HMODULE): M.BOOL
GetModuleHandleExW: [W.BOOL, [W.DWORD, W.LPCTSTR, W.HMODULE] ],

Should we use this ?

GetModuleHandleExW(dwFlags: M.DWORD, lpModuleName: M.LPCTSTR | null, phModule: Buffer): M.BOOL
GetModuleHandleExW: [W.BOOL, [W.DWORD, W.LPCTSTR, 'pointer'] ],

I prefer the first, the param handle/hmodule etc, somewhere is input param( as number), somewhere is output param(as buffer/pointer).

Venryx commented 4 years ago

pls try next version win32-api@^9.0.0 type HANDLE changed to number | bigint | string (uint64/uint32)

Great! Thanks for your work on this issue, despite my limited description/grasp of it.

When I have time to try out the new version, I'll plug it into my project (removing my old modifications), and let you know if it works as expected.

Should we use this ? [...]

I prefer the first, the param handle/hmodule etc, somewhere is input param( as number), somewhere is output param(as buffer/pointer).

I looked the file and description over for a bit, but to be honest, I am not really sure what the difference between the two approaches are. Partly it's a language barrier, but mostly it's just that my familiarity with pointers/buffers/etc for the Windows API is low, so I don't know, for example, how M.HMODULE differs from a Buffer or pointer.

I understand the basic concept of a pointer (theoretically, and practically to some extent, as I had to in the past to get some Windows API calls to work using ffi), and also understand some of how Buffer works (it's a special "shapeless" Javascript construct which is linked to a stable point in unmanaged memory, with ffi providing functions to read and write to that unmanaged memory), but I haven't worked with them enough to easily grasp what the specific code elements Buffer, "pointer", and M.HMODULE specifically represent in this context.

In summary: I don't understand the difference between the two, so just do what you think is best for now. 😆

waitingsong commented 2 years ago

Value of handles has been changed to uin32 or uint64. so close it ~~