mapowscommon.c: dimensions argument needs to be a string

[warmerdam]

The integer dimensions variable is passed to xmlNewChild() in two places in mapowscommon.c where I think it should be a string (as suggested by compiler warnings). I have corrected this in trunk as r6981, but I think this change should also be ported into the 5.0 stable branch.

While I am not using SOS and didn't experience any crashes, it *seems* like this could easily result in an access violation or at least an improper XML document being produced.

Tom - could you review, and if you agree port this back into 5.0 stable?

[tomkralidis]

Frank: What kind of warnings? I never got compiler warnings (FC7) when initially implementing this. What kind of situation would cause an access violation or malformed XML document (I'm having trouble understanding)?

int dimensions is something that would always be set by the calling code, and would always (in MapServer's case) be a single digit.

[warmerdam]

Tom,

The warning looks like this:

mapowscommon.c: In function `msOWSCommonBoundingBox':
mapowscommon.c:522: warning: cast to pointer from integer of different size
mapowscommon.c: In function `msOWSCommonWGS84BoundingBox':
mapowscommon.c:562: warning: cast to pointer from integer of different size

The problem is that "dimensions" is an "int" in the following call, but xmlNewProp() expects a string as the last argument.

  xmlNewProp(psRootNode, BAD_CAST "dimensions", BAD_CAST dimensions);

The warning is that BAD_CAST is making a pointer from the integer dimensions. Actually, when I consider it, I get the warning because I am building on a 64bit system. But the underlying problem is very real. If dimensions is 2, then it will be cast to a string pointer, pointing to the member at memory address 0x00000002. If you review the patch, you will see I prepare a string with the number in it, and pass that.

I'm not clear on why you use the BAD_CAST macro. In this case it is defeating normal C type checking, which is why you never saw the warning/error you should have. In my work on mapwcs.c I find I don't need the BAD_CAST macro.

[tomkralidis]

Ah, ok. The BAD_CAST thing. We went this way (frrom the xmlsoft.org examples) initially to build mapowscommon.c and mapogcsos.c without warnings. Daniel also mentioned concern over this in the past.

I'd like to move away from using BAD_CAST. Once I see your mapwcs.c (or even an example snippet), I can work on removing this from the codebase.

Thanks alot for the helpful explanation. I'll 5.0 backport this.

[tomkralidis]

Done in r6985. Marking as fixed. Props to Frank for helping me out here.