Visual Studio and RAD Studio 32 and 64 bit compilation issues #22

Jeanmilost · 2022-01-15T18:17:26Z

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

…sues

- fixed: unreachable code

matthieu-labas · 2022-01-19T10:39:02Z

src/sxmlc.c

+	dsb.buf_len = buffer_len;
+	dsb.cur_pos = 0;
+#else
+	DataSourceBuffer dsb = { buffer, buffer_len, 0 };


I understand this does not compile with RAD Studio? Which C version does the compiler use?

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 :-)

matthieu-labas · 2022-01-19T10:40:16Z

src/sxmlc.c

+#else
+	DataSourceBuffer dsb = { buffer, buffer_len, 0 };
+
+	#ifdef _MSC_VER


I understand this is specific to MS Visual Studio. Does it complain about a structure not being initialized?

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.

matthieu-labas · 2022-01-19T10:43:14Z

src/sxmlc.c

@@ -2178,7 +2187,7 @@ int read_line_alloc(void* in, DataSourceType in_type, SXML_CHAR** line, int* sz_
 			} else
 				*line = pt;
 		}
-		if (sz_line && n < *sz_line)
+		if (line && sz_line && n < *sz_line)


Which version of sxmlc are you using? In the latest (4.5.1), line cannot be NULL here.

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

NOTE I'm compiling in 64 bit, it's may be the reason why I receive many more warnings than other users ;-)

matthieu-labas · 2022-01-19T10:46:23Z

src/sxmlc.c

@@ -334,13 +334,21 @@ int XMLNode_copy(XMLNode* dst, const XMLNode* src, int copy_children)

 	/* Tag */
 	if (src->tag != NULL) {
+	#ifdef _MSC_VER
+		dst->tag = _strdup(src->tag);


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.

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.

matthieu-labas · 2022-01-19T10:52:28Z

src/sxmlc.c

@@ -1752,7 +1826,11 @@ int XMLDoc_parse_file_SAX(const SXML_CHAR* filename, const SAX_Callbacks* sax, v
 	if (sax == NULL || filename == NULL || filename[0] == NULC)
 		return FALSE;

+#ifdef _MSC_VER
+	fopen_s(&f, filename, fmode);


In my understanding fopen_s() is not available everywhere, so why use it here?

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

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...

matthieu-labas · 2022-09-02T09:04:42Z

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):

upper-cased true and false. I kept the lower-case as well for compatibility
added an MS specific #define to handle sx_strdup

Unfortunately, I couldn't handle the strncpy_s cases easily; but I'll definitely look into it because it's safer. The function seems to be available in standard C but it doesn't show in my Linux. I'll look into that some other time.

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 strncpy_s and others. If it's something you want to improve in sxmlc, please do so :) but prototypes should be defined in sxmlc.h (with sx_ prefix) and harmonized throughout the code, keeping in mind most people use it in embedded software running Linux :)

Regards,
Matthieu

Jeanmilost added 3 commits December 9, 2021 20:54

- fixed: several Visual Studio 2019 and 64 bit compilation related is…

0d1fe58

…sues

- fixed: incompatibilities with Visual Studio 2019

335140a

- fixed: RAD studio XE7 compilation issue

d967ce3

- fixed: unreachable code

matthieu-labas reviewed Jan 19, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Visual Studio and RAD Studio 32 and 64 bit compilation issues #22

Visual Studio and RAD Studio 32 and 64 bit compilation issues #22

Jeanmilost commented Jan 15, 2022

matthieu-labas Jan 19, 2022

Jeanmilost Jan 20, 2022

matthieu-labas Jan 19, 2022

Jeanmilost Jan 20, 2022

matthieu-labas Jan 19, 2022

Jeanmilost Jan 20, 2022

Jeanmilost Jan 20, 2022

matthieu-labas Jan 19, 2022 •

edited

Loading

Jeanmilost Jan 20, 2022 •

edited

Loading

matthieu-labas Jan 19, 2022

Jeanmilost Jan 20, 2022

matthieu-labas left a comment

matthieu-labas commented Sep 2, 2022

Visual Studio and RAD Studio 32 and 64 bit compilation issues #22

Are you sure you want to change the base?

Visual Studio and RAD Studio 32 and 64 bit compilation issues #22

Conversation

Jeanmilost commented Jan 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthieu-labas Jan 19, 2022 • edited Loading

Choose a reason for hiding this comment

Jeanmilost Jan 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthieu-labas left a comment

Choose a reason for hiding this comment

matthieu-labas commented Sep 2, 2022

matthieu-labas Jan 19, 2022 •

edited

Loading

Jeanmilost Jan 20, 2022 •

edited

Loading