Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix fallout from removal of ibize_platform.h #658

Merged
merged 3 commits into from May 13, 2015
Merged

fix fallout from removal of ibize_platform.h #658

merged 3 commits into from May 13, 2015

Conversation

smcv
Copy link
Contributor

@smcv smcv commented Apr 27, 2015

After commit 0323591 removed the inclusion of ibize_platform.h, I get compiler errors in codeJK2 on Linux (gcc).

One of the commits here should also fix #651.

@smcv smcv mentioned this pull request Apr 27, 2015
Closed
@ensiform
Copy link
Member

Why was cstdint specifically added to JK2 ?

@smcv
Copy link
Contributor Author

smcv commented Apr 27, 2015

Why was cstdint specifically added to JK2 ?

I'm not sure why specifically cstdint. The high-level reason was to get uint32_t defined so that ParseRGB() could return one instead of a (typedef for a) DWORD, when the single #include of ibize_platform.h was removed from JK2: nothing else depended on that file, or on anything else from tools, so getting rid of that #include was a good thing overall.

The rest of the codebase uses the C stdint.h to get the [u]intN_t family of types. That is, strictly speaking, not portable (it isn't in the subset of C that C++ compilers guarantee to provide, but in practice they all do); cstdint is the standard way to get uint32_t defined by a C++ compiler.

ensiform added a commit that referenced this pull request Apr 30, 2015
@ensiform
JK2: Remove cstdint usage from icarus/tokenizer.h.
38aa581 
It doesn't seem to be necessary right now.
Switched usage to `unsigned int` as it is in the JA MP version of the file.

Refs #651 #658
@smcv
codeJK2: replace __cdecl with QDECL
8f8bad9 
On Windows, QDECL expands to __cdecl, which is presumably necessary.
On other platforms, the __cdecl decoration does not exist, and
QDECL correctly expands to nothing.

Before commit 0323591, __cdecl expanded to nothing on non-Windows,
due to inclusion of ibize_platform.h.
@smcv
SP, JK2: g_savegame: silence compiler warning
905b531 
.../code/game/g_savegame.cpp: In function ‘void
EnumerateFields(const save_field_t*, const byte*, unsigned int, int)’:
.../code/game/g_savegame.cpp:512:22: warning: comparison between
signed and unsigned integer expressions [-Wsign-compare]
    assert(pField->iOffset < iLen);
                      ^

iLen is a non-negative amount of memory, so size_t is a more
appropriate type than int.
@smcv
JK2: icarus: #include <string.h> for strcmp, memcpy in inline functions
7eb715d 
lessstr::operator() in tokenizer.h calls strcmp(), and
CBlockMember::WriteDataPointer<T> calls memcpy(). These were presumably
declared via some other header in older OpenJK or in older g++.
@smcv
Copy link
Contributor Author

smcv commented Apr 30, 2015

codeJK2: replace __cdecl with QDECL is a remaining issue related to disentangling JK2 from ibize_platform.h.

The other two commits here are simple fixes for compiler warnings (g_savegame...) and errors (icarus...) which might be related to removal of that header, or not - I'm not sure. I can do a separate pull request per fix if you want, but that's probably more time consuming (for you and for me), so I'd rather continue to group simple fixes together.

I've dropped the #651 part from this pull request, since removing the use of cstdint downgrades its severity from "can't compile without fixing this" to "would be nice to fix one day".

@smcv smcv mentioned this pull request Apr 30, 2015
Merged
xycaleth added a commit that referenced this pull request May 13, 2015
@xycaleth
Merge pull request #658 from smcv/ibize-removal
5edc2dc 
fix fallout from removal of ibize_platform.h
@xycaleth xycaleth merged commit 5edc2dc into JACoders:master May 13, 2015
@smcv smcv deleted the ibize-removal branch July 29, 2015 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SP won't compile on C++14
3 participants
@smcv @ensiform @xycaleth