Improve readibility and performance of msCopyMap

[sgillies@…]

Move to use of macros rather than copyProperty function as suggested by
Mladen Turk.

[sgillies@…]

MT's new mapcopy.c file

[sgillies@…]

MT, I am experiencing a problem with your MS_COPYSTELEM macro.  Every time it
is used, I get a warning:

  mapcopy.c:160:23: warning: pasting "->" and "type" does not give a
  valid preprocessing token

and so on.  I'm using gcc 3.2.2.  You mentioned that your work is .NET oriented,
so you must be using MSVC?  We should find something that will work across 
platforms even if it's a bit less concise.

Also, re your move of msCopySymbol from mapsymbol.c -> mapcopy.c ... that is
where it started, but we are trying organize the code better and put all 
the symbol stuff together in one place.  Since the map copying is fragile, it
seems like keeping it nearer to the other object functions will remind 
developers to keep it updated if they make changes to the symbolObj structure.

[mturk@…]

The promlem is with the 'token-pasting' operator (##) in macro
MS_COPYSTELEM. VC7 allows to be the first token in join, but seems that gcc 
don't.

The solution is to replace all ocureneces of MS_COPYSTELEM(xxxx) with
dst->xxxx = src->xxxx;

I'll make a script and replace them all.

Regarding msCopySymbol, that's fine, but in that case the (working) macros will 
have to go in map.h.

[sgillies@…]

The macro that works for me is 

  MS_COPYSTELEM(x) dst->/**/x = src->/**/x

That's the gcc traditional form.  The ISO macro you wrote *should* work.
Can you think of a good way to detect which one should be used?  I think
that such a macro is useful.

How about putting the macros in a new mapcopy.h instead of map.h?

[sgillies@…]

Actually, what I meant to write is that the ISO macro *should not* work, even
for MSVC, because it is supposed to join two valid tokens and neither src->
or dst-> are valid tokens.

[mturk@…]

Patched mapcopy.c

[mturk@…]

Sure, the
#define MS_COPYSTELEM(name) (dst)->/**/name = (src)->/**/name

works both on
gcc (GCC) 3.2.3 (mingw special 20030504-1)
and MSVC.

Seems that I've run 'sed' prior seeing your comments.
The macro is quite good cause it resolves typo errors.

 

[mturk@…]

*** Bug 700 has been marked as a duplicate of this bug. ***

[sgillies@…]

OK, I have added the macros to mapcopy.h and am now using them in all the
msCopy* functions.

There are a couple of odd cases where the MS_COPYSTRING by itself was not
robust: 1) the elements of the args member of a projectionObj, 2) the
symbolname member of a symbolObj.  You can see how I worked around in
mapsymbol.c:msCopySymbol() and mapcopy.c:msCopyProjection().

The testcopy program runs through valgrind with no errors and all the
Python unit tests, including new cloning unit tests, pass.

Looking forward to your feedback, MT.  And thanks for the input, I think that
resolving this bug (and 640) makes the software a lot better.  

[sgillies@…]

changes committed to 4.2 branch as well as main.

[mturk@…]

Just saw your comments on few problems with macros.
Perhaps the problem might be resolved by prefixing macro operators with 
underscore. It will in that case resolve common naming conflicts, cause 'name' 
is quite common variable name, as well as dst and src.

Also the MS_COPYSTRING macro has a bug for array copying, cause the
'if (src)' shoude be 'if ((src))'.
Sorry, but I've miss that.

Here is the slightly better version:

#define MS_COPYSTELEM(_name) (dst)->/**/_name = (src)->/**/_name

#define MS_MACROBEGIN              do {
#define MS_MACROEND                } while (0) 

#define MS_COPYRECT(_dst, _src)         \
    MS_MACROBEGIN                       \
        (_dst)->minx = (_src)->minx;    \
        (_dst)->miny = (_src)->miny;    \
        (_dst)->maxx = (_src)->maxx;    \
        (_dst)->maxy = (_src)->maxy;    \
    MS_MACROEND

#define MS_COPYPOINT(_dst, _src)        \
    MS_MACROBEGIN                       \
        (_dst)->x = (_src)->x;          \
        (_dst)->y = (_src)->y;          \
        (_dst)->m = (_src)->m;          \
    MS_MACROEND

#define MS_COPYCOLOR(_dst, _src)        \
    MS_MACROBEGIN                       \
        (_dst)->pen   = (_src)->pen;    \
        (_dst)->red   = (_src)->red;    \
        (_dst)->green = (_src)->green;  \
        (_dst)->blue  = (_src)->blue;   \
    MS_MACROEND

#define MS_COPYSTRING(_dst, _src)       \
    MS_MACROBEGIN                       \
        if ((_dst) != NULL)             \
            msFree((_dst));             \
        if ((_src))                     \
            (_dst) = strdup((_src));    \
        else                            \
            (_dst) = NULL;              \
    MS_MACROEND


 

[sgillies@…]

Have committed the changes to CVS, 4.2 and main.

[sgillies@…]

Closing this bug.  Any problems with msCopyMap will be a new issue.