POINTTOPOINTS shouldn't cast result of MAKELONG to POINTS, The result should be of type LONG. Attached test proves that. It compiles on MSVC, but doesn't on MinGW. Removing cast in POINTTOPOINTS define fixes the problem.
Thanks for the report. I agree that there is a problem here; however, your analysis and proposed solution seem flawed.
According to http://msdn.microsoft.com/en-us/library/dd162810\(VS.85).aspx, the result of POINTTOPOINTS is a POINTS struct. Simply removing the cast from the present definition makes the result a LONG, which is incompatible with the required POINTS struct; changing line 5 of your test case from
LONG l = POINTTOPOINTS(p);
to
POINTS l = POINTTOPOINTS(p);
(as it should be for correct usage), proves that your proposed solution is inappropriate, (notwithstanding that MSVC apparently, and improperly IMO, appears to compile your sample code).
Thanks for the answer. I believe that MSDN is wrong (again). My test compiles on MSVC because the result is LONG in MS SDK and that's also what Mozilla code (which I was trying to compile when I've found this bug) assumes as well. It's used to pass POINT structure to PostMessage. Given that, your change won't help.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
currently doesn't compile, in spite of its conformity with MSDN documented usage.
I don't accept OP's contention that MSDN is wrong; such a contention is utterly illogical. If mozilla expects POINTTOPOINTS to return a LONG, in spite of MSDN stating, entirely logically, that it returns a POINTS struct, then that is a bug in mozilla's code. However, the inability to compile the above code, expressing MSDN conformant usage, also represents a bug in our winuser.h; my suggested reformulation, above, fixes it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I don't accept OP's contention that MSDN is wrong; such a contention is utterly illogical.
I'm Wine developer and we need to deal with more than just prototypes of MSDN content everyday. You're not going to convince me to trust MSDN after seeing it being incredibly wrong in so many cases. MS implementation is based on their SDK and that SDK is what defines Win32 API. MSDN is just its (bad) documentation. If SDK is different than MSDN, then SDK is what matters. You're trying to tell me that you know better than MS (who wrote the SDK) what this macro does based on MS documentation... That's illogical.
If mozilla expects POINTTOPOINTS to return a LONG, in spite of MSDN stating,
entirely logically, that it returns a POINTS struct, then that is a bug in
mozilla's code.
What would you expect them to do? Write a code that doesn't compile on MSVC, but agrees with this MSDN statement?
However, the inability to compile the above code, expressing MSDN conformant
usage, also represents a bug in our winuser.h; my suggested reformulation,
above, fixes it.
I don't care anymore. I ended up learning about mingw-w64 and became their developer now. Mozilla compiles fine both on MSVC and mingw-w64 (and it's compiled this way by distros for Wine Gecko package). There are currently so many missing pieces in plain mingw.org, that it's hopeless to even try Mozilla codebase compilation.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I've decided to remove the cast regardless of MSDN. User experience has proven the point and removing the cast allows the user the privilege to cast to the POINTS structure themselves or to use the MAKELONG result directly.
I've put this in the doc status because I reference this ticket in the comments for POINTTOPOINTS.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I've decided to remove the cast regardless of MSDN.
So, in reality you've implemented it as POINTTOLONG; (it converts from a POINT struct to a long, not to the POINTS struct which MSDN would lead us to expect).
User experience has proven the point and removing the cast allows the user the privilege to cast to the POINTS structure themselves or to use the MAKELONG result directly.
Maybe so, but it is impossible to simply cast a long to a POINTS struct, (or indeed, to any other aggregate type); such a cast is invalid, and neither gcc nor g++ will allow it.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Maybe so, but it is impossible to simply cast a long to a POINTS struct, (or indeed, to any other aggregate type); such a cast is invalid, and neither gcc nor g++ will allow it.
Then that makes the change even more pertinent.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
In which case, how do you convert from a POINT to a POINTS struct? It never worked in the past; POINTTOPOINTS sure as hell still won't do it.
The OP's "proof" didn't prove his case, since his assertion was bogus in the first place; (I can easily submit any number of bogus proofs of any bogus assertion). Of course, it may be that Microsoft's implementation of POINTTOPOINTS is every bit as badly broken as ours remains, or it may be that MSVC is just so badly broken itself, that it implicitly permits the invalid cast, but let's not say that this is fixed, because it utterly fails to deliver on its obvious, as documented intent.
Let us rather say that it remains broken, albeit in a manner which is more consistent with Microsoft's own broken implementation.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
test
Thanks for the report. I agree that there is a problem here; however, your analysis and proposed solution seem flawed.
According to http://msdn.microsoft.com/en-us/library/dd162810\(VS.85).aspx, the result of POINTTOPOINTS is a POINTS struct. Simply removing the cast from the present definition makes the result a LONG, which is incompatible with the required POINTS struct; changing line 5 of your test case from
LONG l = POINTTOPOINTS(p);
to
POINTS l = POINTTOPOINTS(p);
(as it should be for correct usage), proves that your proposed solution is inappropriate, (notwithstanding that MSVC apparently, and improperly IMO, appears to compile your sample code).
Surely, something like
#define POINTTOPOINTS(p) (POINTS){ (SHORT)((p).x), (SHORT)((p).y) }
is what is required? (And for some symmetry, in spite of the intrinsic asymmetry of the prototypes, maybe we could also redefine
#define POINTSTOPOINT(p,ps) (p) = (POINT){ (LONG)((ps).x), (LONG)((ps).y) }
rather than the present hideous address/pointer dereferencing kludge)?
Thanks for the answer. I believe that MSDN is wrong (again). My test compiles on MSVC because the result is LONG in MS SDK and that's also what Mozilla code (which I was trying to compile when I've found this bug) assumes as well. It's used to pass POINT structure to PostMessage. Given that, your change won't help.
Keith, should I look into this one?
Last edit: Keith Marshall 2013-02-07
Something certainly needs to be done; the code:
currently doesn't compile, in spite of its conformity with MSDN documented usage.
I don't accept OP's contention that MSDN is wrong; such a contention is utterly illogical. If mozilla expects POINTTOPOINTS to return a LONG, in spite of MSDN stating, entirely logically, that it returns a POINTS struct, then that is a bug in mozilla's code. However, the inability to compile the above code, expressing MSDN conformant usage, also represents a bug in our winuser.h; my suggested reformulation, above, fixes it.
I'm Wine developer and we need to deal with more than just prototypes of MSDN content everyday. You're not going to convince me to trust MSDN after seeing it being incredibly wrong in so many cases. MS implementation is based on their SDK and that SDK is what defines Win32 API. MSDN is just its (bad) documentation. If SDK is different than MSDN, then SDK is what matters. You're trying to tell me that you know better than MS (who wrote the SDK) what this macro does based on MS documentation... That's illogical.
What would you expect them to do? Write a code that doesn't compile on MSVC, but agrees with this MSDN statement?
I don't care anymore. I ended up learning about mingw-w64 and became their developer now. Mozilla compiles fine both on MSVC and mingw-w64 (and it's compiled this way by distros for Wine Gecko package). There are currently so many missing pieces in plain mingw.org, that it's hopeless to even try Mozilla codebase compilation.
I've decided to remove the cast regardless of MSDN. User experience has proven the point and removing the cast allows the user the privilege to cast to the POINTS structure themselves or to use the MAKELONG result directly.
I've put this in the doc status because I reference this ticket in the comments for POINTTOPOINTS.
So, in reality you've implemented it as POINTTOLONG; (it converts from a POINT struct to a long, not to the POINTS struct which MSDN would lead us to expect).
Maybe so, but it is impossible to simply cast a long to a POINTS struct, (or indeed, to any other aggregate type); such a cast is invalid, and neither gcc nor g++ will allow it.
Then that makes the change even more pertinent.
In which case, how do you convert from a POINT to a POINTS struct? It never worked in the past; POINTTOPOINTS sure as hell still won't do it.
The OP's "proof" didn't prove his case, since his assertion was bogus in the first place; (I can easily submit any number of bogus proofs of any bogus assertion). Of course, it may be that Microsoft's implementation of POINTTOPOINTS is every bit as badly broken as ours remains, or it may be that MSVC is just so badly broken itself, that it implicitly permits the invalid cast, but let's not say that this is fixed, because it utterly fails to deliver on its obvious, as documented intent.
Let us rather say that it remains broken, albeit in a manner which is more consistent with Microsoft's own broken implementation.
Notes added to the NEWS file.