Made ExternMFD to Work #66
Conversation
There was a problem hiding this comment.
This looks good, my only concern is the change of data types in the API interface functions. for example UINT Id() const -> INT_PTR Id() const in MFDAPI.h.
INT_PTR resolves to int for 32-bit builds while UINT is unsigned int. Wouldn't this break backward compatibility?
|
I don't know about the backwards compatibility since, isn't that value frequently passed to a functions that take "int" as an input type. In a case of ExternMFD that value is a pointer to ExternMFD and if the value is high enough to become a negative integer then a CTD is a result with current code. Of course, one possibility it translate all MFD Id's to a DWORD and/or DWORD_PTR. |
|
Isn't |
|
Technically yes but the return value is converted to "int" eventually and the "int" is used to represent the Id parameter almost everywhere else in the code base. Should be noted that at-least RepaintMFDButtons() crashes if receives a negative id. But that's easy to fix. But, if the API compatibility with existing addons is in question then the function could just return UINT_PTR and convert it to INT_PTR later on, or replace all related "int's" with "uint's" |
|
In that case I would vote for UINT_PTR. This would make sure that the behaviour for 32-bit builds remains the same and existing addons wouldn't be confronted with an interface change. If the Id value is considered unsigned in the API, then converting it to signed later on is probably an error and should be addressed there. |
|
It would seem that the rest of the API is using "int" type as an example oapiRegisterMFD() etc.. So, it would be practical to keep the "int" and "INT_PTR" in place and use the "UINT_PTR" as return type for ExterMFD::Id() and let C++ to convert it to "int" or "INT_PTR" as needed. I have also added ASSERT() statement. That code section doesn't seem to be in use. If it's used in some rare cases then we have to figure out something. |
No description provided.