libgis: Add string concatenation function

[nilason]

Working on solutions for compiler warnings (#1247), specifically attempting to address a -Wformat-overflow I missed the existence of a string concatenation function.

This PR is a suggestion for a concatenation function added to libgis:

char *G_str_concat(const char **src_strings, int num_strings, const char *sep, int maxsize);

It is based on a local version of the memccpy implementation taken from www.open-std.org : N2349.

The PR consists of two commits, one with the addition of the G_str_concat() function, the other is a fix for the mentioned warning (and serves as usage example).

This function could possibly find its use elsewhere, e.g. r.li.cwed has its own concatenation solution, which may justify the addition.

[nilason]

centos7 isn't too happy with this:

strings.c:30:40: error: expected ';', ',' or ')' before 'dst'
 static void *G__memccpy(void *restrict dst, const void *restrict src, int c, size_t n);
                                        ^
strings.c:71:33: error: expected ';', ',' or ')' before 'dst'
 void *G__memccpy(void *restrict dst, const void *restrict src, int c, size_t n)
                                 ^

??
(restrict is a C99 feature...)

[HuidaeCho]

centos7 isn't too happy with this:

strings.c:30:40: error: expected ';', ',' or ')' before 'dst'
 static void *G__memccpy(void *restrict dst, const void *restrict src, int c, size_t n);
                                        ^
strings.c:71:33: error: expected ';', ',' or ')' before 'dst'
 void *G__memccpy(void *restrict dst, const void *restrict src, int c, size_t n)
                                 ^

??
(restrict is a C99 feature...)

Should we support compiler-specific extensions like __restrict__? Does it mean GCC (or Clang for that matter) does not support C99 (or even C11?) fully or why did they name restrict that way? Just curious.

Reading https://trac.osgeo.org/grass/wiki/RFC/7_LanguageStandardsSupport, have we already reached a consensus about the final language standards?

[nilason]

I was trying to please Centos7 CI with __restrict__, as it didn't work with restrict. It uses gcc 4.8.5 and should probably work with -std=c99. Now, this detail (restrict) isn't really a necessity for the function.

[HuidaeCho]

I was trying to please Centos7 CI with __restrict__, as it didn't work with restrict. It uses gcc 4.8.5 and should probably work with -std=c99. Now, this detail (restrict) isn't really a necessity for the function.

I agree. __restrict__ is not necessary for the function. If anything, it would be a programmer's (our) mistake.

[nilason]

I was trying to please Centos7 CI with __restrict__, as it didn't work with restrict. It uses gcc 4.8.5 and should probably work with -std=c99. Now, this detail (restrict) isn't really a necessity for the function.

By default gcc 4.8.5 uses -std=gnu90.

For the record: configuring CentOS7 with -std=gnu99, enables the use of restrict.

[nilason]

There is memccpy(3) in POSIX. Is there any specific reason for adding our own version of this function?

As I mentioned before, the name memccpy (not the function) has been deprecated with VS 2019 in favour for _memccpy. That leaves us in the future with a deprecation warning (no warnings on CI Windows build just yet) or some preprocessor acrobatics to avoid it. If you have a suggestion on this I'm happy to drop the local implementation of memccpy.

[nilason]

@HuidaeCho Thanks ! Applied you suggestions, ready for a new round :) .

[HuidaeCho]

@HuidaeCho Thanks ! Applied you suggestions, ready for a new round :) .

@nilason Looks good to me. Just out of curiosity, any reason for double underscores in G__memccpy? The same reason why MS renamed their memccpy to _memccpy? That reasoning may not apply in our code base.

@neteler Do we have a rule for G_name() vs. G__name()? Higher-level vs. lower-level? User developers vs. core developers?

[nilason]

@nilason Looks good to me. Just out of curiosity, any reason for double underscores in G__memccpy?

It's just to differentiate against open API.

[HuidaeCho]

@nilason Looks good to me. Just out of curiosity, any reason for double underscores in G__memccpy?

It's just to differentiate against open API.

Oh! That's a static function.

[HuidaeCho]

@nilason I just have one last suggestion. Please move G__memccpy() and its prototype to the top. The other functions are more for direct string manipulations than memory copy.

[nilason]

Reordered per suggestion.

[HuidaeCho]

Reordered per suggestion.

A separate PR per https://trac.osgeo.org/grass/wiki/Submitting/C#Indentation would be great or maybe together in this PR may be better.

[nilason]

I applied the grass_indent.sh, but kept its result only on touched parts (not whole file). That should be for another time/PR.

If I only knew before, there is no need for those 8-sized tabs...

[nilason]

Thanks a lot @HuidaeCho !
Cherry picked and pushed manually to keep the commits separate.