| msg115701 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-06 12:55 |
| I'm posting this in case it is a sign of a problem. Apparently some variable named "hf" is an INT_PTR used as an int (according to Visual Studio), but "hf" doesn't seem to be defined or declared in _msi.c at all. 12>..\PC\_msi.c(66) : warning C4244: 'function' : conversion from 'INT_PTR' to 'int', possible loss of data 12>..\PC\_msi.c(74) : warning C4244: 'function' : conversion from 'INT_PTR' to 'int', possible loss of data 12>..\PC\_msi.c(82) : warning C4244: 'function' : conversion from 'INT_PTR' to 'int', possible loss of data 12>..\PC\_msi.c(90) : warning C4244: 'function' : conversion from 'INT_PTR' to 'int', possible loss of data |
|
|
| msg115750 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-09-07 11:15 |
| FNFCIREAD &co are macros to help the definition of callback functions: http://msdn.microsoft.com/en-us/library/ff797940.aspx "hf" is defined as INT_PTR, but the value it receives is the result of FNFCIOPEN(), which fits in int. It is safe to cast "hf" to an int if you want to disable the warning. |
|
|
| msg116191 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2010-09-12 13:37 |
| It's probably best to rewrite these functions in the way the SDK Example section works (i.e. CreateFile/ReadFile) instead of _open/_read (*). As Amaury says, the warnings are harmless: the file numbers will always be in range, as _open created them that way (in fact, they will most likely be below 20 or so). (*) They must have changed the SDK docs over time; IIRC, I already copied the current text (using _open) from the SDK docs back then. |
|
|
| msg117256 - (view) |
Author: Jon Anglin (janglin) |
Date: 2010-09-24 02:30 |
| .diff contains a patch that compiles clean on 32 and 64 bit Windows. This patch is exactly what Amaury Forgeot d'Arc recommended in . |
|
|
| msg117316 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2010-09-24 17:52 |
| -1 for the patch. I still maintain that it is better to follow the current SDK. |
|
|
| msg117341 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * |
Date: 2010-09-24 22:42 |
| Which "SDK Example" are you referring to? I could not find any example. |
|
|
| msg117342 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2010-09-24 22:54 |
| See, for example, the link you gave in . It has this sample code in it (listed in the "Examples" section): FNFCIREAD(fnFileRead) { DWORD dwBytesRead = 0; UNREFERENCED_PARAMETER(pv); if( ReadFile((HANDLE)hf, memory, cb, &dwBytesRead, NULL) == FALSE ) { dwBytesRead = (DWORD)-1; *err = GetLastError(); } return dwBytesRead; } |
|
|
| msg117343 - (view) |
Author: Jon Anglin (janglin) |
Date: 2010-09-25 01:10 |
| Martin Lowis do you mean API when you type SDK? If I understand what you are saying, you would rather use the Win32 API instead of the CRT API? It may interest you to know that _open calls CreateFile internally, _read calls ReadFile, _write calls WriteFile, _lseek calls SetFilePointer, and _close calls CloseHandle. There is a bit more to it than that but it is not really relevant to this discussion. What is relevant is that inside _open, CreateFile will return an OS HANDLE type (64 bits in our case) that is mapped to a 32 bit integer CRT file descriptor that is then returned. The other functions such as _read, etc…, will look up the 64 bit OS HANDLE value from the given 32 bit file descriptor and call the corresponding Win32 API function. We could rewrite the functions using the Win32 API directly but we don’t have to. I realize this is a Windows only module but the use of the CRT API is more familiar to a majority of the Python developers (I would guess). I stand by the patch. |
|
|
| msg117350 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2010-09-25 06:35 |
| > Martin Lowis do you mean API when you type SDK? When I said "SDK examples", then I really meant "examples as published in the SDK", not "examples as published in the API" (and the SDK documentation, in turn, is published on msdn). But yes, the SDK examples use Win32 directly. > If I understand what you are saying, you would rather use the Win32 > API instead of the CRT API? Correct. > It may interest you to know that _open calls CreateFile internally, > _read calls ReadFile, _write calls WriteFile, _lseek calls > SetFilePointer, and _close calls CloseHandle. I'm fully aware of that. > We could rewrite the functions using the Win32 API directly but we > don’t have to. I realize this is a Windows only module but the use > of the CRT API is more familiar to a majority of the Python > developers (I would guess). I have the long-term plan to eliminate all CRT usage from Python on Windows. In this case, there is a straight-forward opportunity to do so. Nothing is really gained from using the CRT (as the cabinet SDK is probably even less familiar to Python developers than CreateFile), plus using the CRT causes compiler warnings, as Microsoft clearly intends that these routines would be implemented using the Windows API directly. |
|
|
| msg117361 - (view) |
Author: Jon Anglin (janglin) |
Date: 2010-09-25 12:53 |
| > I have the long-term plan to eliminate all CRT usage from Python > on Windows. In this case, there is a straight-forward opportunity > to do so. Oh, OK. If that is the plan then I am on board. I will re-code the patch using the Win32 API directly. >> It may interest you to know that _open calls CreateFile internally, > I'm fully aware of that. I meant no disrespect, I just didn't know if you were a "Windows" guy. If you asked me what system call _open (or others) calls on Linux or Mac, I would have no clue. |
|
|
| msg117696 - (view) |
Author: Jon Anglin (janglin) |
Date: 2010-09-30 03:51 |
| I have uploaded another patch that replaces CRT API calls with Win32 API calls. It compiles cleanly under 32 and 64 bit Windows. Is there a unit test for msilib? I was not able to find one, thus the patch is not fully tested. |
|
|
| msg221138 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-06-21 00:42 |
| Could somebody review the patch please as it's well over my head. |
|
|
| msg237053 - (view) |
Author: Steve Dower (steve.dower) * |
Date: 2015-03-02 15:53 |
| We'd need a contributor agreement from Jon anyway, so unless he rejoins this thread and is willing to sign that, there's little point looking at the patch. |
|
|