Visual Studio and RAD Studio 32 and 64 bit compilation issues#22
Visual Studio and RAD Studio 32 and 64 bit compilation issues#22Jeanmilost wants to merge 3 commits into
Conversation
Jeanmilost
commented
Jan 15, 2022
Jeanmilost
commented
- fixed: 64 bit compilation issues
- fixed: project could not be compiled with Visual Studio 2019
- fixed: project could not be compiled with Embarcadero RAD Studio XE7
| dsb.buf_len = buffer_len; | ||
| dsb.cur_pos = 0; | ||
| #else | ||
| DataSourceBuffer dsb = { buffer, buffer_len, 0 }; |
There was a problem hiding this comment.
I understand this does not compile with RAD Studio? Which C version does the compiler use?
There was a problem hiding this comment.
In fact this code isn't working with Embarcadero RAD studio XE7. It's the reason for the #ifdef CODEGEARC instead of #ifdef _MSC_VER, as for the other ifdefs. On the other hands I compiled this code without issue with Visual Studio 2019, Code::Blocks and xCode :-)
| #else | ||
| DataSourceBuffer dsb = { buffer, buffer_len, 0 }; | ||
|
|
||
| #ifdef _MSC_VER |
There was a problem hiding this comment.
I understand this is specific to MS Visual Studio. Does it complain about a structure not being initialized?
There was a problem hiding this comment.
The reason why I added this ifdef (as well as some similar in several other locations) is that I have a huge list of warnings claimed by Visual Studio otherwise, saying more or less that this value may be used without be initialized. It was somewhat annoying for me, because this drowned out other important warnings I should not miss.
| *line = pt; | ||
| } | ||
| if (sz_line && n < *sz_line) | ||
| if (line && sz_line && n < *sz_line) |
There was a problem hiding this comment.
Which version of sxmlc are you using? In the latest (4.5.1), line cannot be NULL here.
There was a problem hiding this comment.
Indeed, and I understand that you're puzzled. I added that because Visual Studio was claiming a Buffer Overrun on this line. All in all this resolved nothing, I just forgotten to remove it, you can thus get rid of that. The warning I receive is the following:
Warning C6386 Buffer overrun while writing to '*line': the writable size is 'sz_linesizeof(SXML_CHAR)' bytes, but '2' bytes might be written. ...\sxmlc.c 2191
There was a problem hiding this comment.
NOTE I'm compiling in 64 bit, it's may be the reason why I receive many more warnings than other users ;-)
| /* Tag */ | ||
| if (src->tag != NULL) { | ||
| #ifdef _MSC_VER | ||
| dst->tag = _strdup(src->tag); |
There was a problem hiding this comment.
Is _strdup() only defined in MS Visual Studio? What is the problem with the standard strdup()?
In general, macro sx_strdup() should be used, and potentially redefined if needed, but I'd prefer not clutter code with too much compiler/IDE specifics when possible.
There was a problem hiding this comment.
The issue here is that Visual Studio 2019 refuses to compile several old standard c functions, because they are subject to buffer overrun and some similar issues. For that reason Microsoft replaced them by their own implementation and they force to use them while you try to compile. Unless you activate an option to turn these errors into warnings in each projects, you cannot compile them because VS2019 generates an error on each of these functions. And I'm not very motivated to do this because it's strongly not recommended. But indeed you're right, modify the macro may be enough if the parameters number is the same between the 2 functions. Good remark.
| return FALSE; | ||
|
|
||
| #ifdef _MSC_VER | ||
| fopen_s(&f, filename, fmode); |
There was a problem hiding this comment.
In my understanding fopen_s() is not available everywhere, so why use it here?
There was a problem hiding this comment.
Same reason as above, it's because Microsoft replaced some standard c functions by their own implementation, and they strongly push to use them instead of standard versions, for security reasons. Especially while you try to compile in 64 bit.
matthieu-labas
left a comment
matthieu-labas
left a comment
There was a problem hiding this comment.
I thought the #ifndef false would not trigger if the compiler already knew the constant. Changing all true/false constants could be done (good job btw!) but I'd rather tweak the compilation options to prevent that if possible...
|
Hi! Sorry for being so late, I was reviewing a few things and here is what I did in the latest version (4.5.2):
Unfortunately, I couldn't handle the Also, I decided not to take into account the RAD-specific setting because it seems the problem comes from the compiler: struct initialization is standard for a long time. I wanted to merge your first commit but cherry-pick on the second one but I couldn't find a way to do it. If you have a chance to check the new version, you'll have to redo the changes for the Regards, |
2 participants
