Borland C compiler doesn't like `randomize` in pngvalid.c

[ctruta]

Although I used to be a Borland compiler user ever since Turbo Pascal 5.5 on DOS, I haven't given a spin to their IDEs for many years recently. But earlier today I did just that. I installed C++Builder Community Edition ("turbo-powered" by Clang) and I noticed that pngvalid.c doesn't compile because random() is a macro defined in their <stdlib.h>. I suppose they're keeping it in for backwards compatibility with their old compilers and libraries, and it only gets out of the way on 64-bit builds and/or when __STDC__ is on.

#ifdef blah...
#define randomize() srand((unsigned)time(NULL))
#endif

This breakage only happens in C mode, because in C++ mode it becomes a nice inline function tucked away in namespace std.

So, @jbowler, when you have a chance, could you please update pngvalid.c to your taste, renaming this https://github.com/pnggroup/libpng/blob/626761b4f5e81bb149d67dbd3913bed446a51e08/contrib/libtests/pngvalid.c#L307 to something else that's more out of the way; say (just a suggestion)

static void
init_random(void *pv, size_t size) 

and then

#define RANDOMIZE(object) init_random(&(object), sizeof (object))

If you're against making a change like this, another alternative would be to enforce compilation in Standard C mode, at least for the Borland compilers (if not more).

[jbowler]

Yes, random is expected to be defined in because I have _DEFAULT_SOURCE and _BSD_SOURCE defined at the top, but the code doesn't use it. random() and srandom() and other things are defined in XOpen, SVID and BSD but I can't see anywhere where pngvalid attempts to use them and I don't see what it has to do with the function "randomize".

pngvalid is C99 because of its use of floating point.

[jbowler]

Oh... ok; they're defining "randomize". Yes, compile with --std=c89 (or c90) will fix it and is required in this case. I think that's good practice anyway; we don't support any pre-ANSI compiler and it would seem that Borland as you are running it is pre-ANSI,

In fact pngvalid.c requires C99 so it should probably be checking STDC anyway. ISO don't define the feature test macro I'm using but they do define STDC.

[jbowler]

PR #603 you need PR #602 to test.

[ctruta]

Oh... ok; they're defining "randomize". Yes, compile with --std=c89 (or c90) will fix it and is required in this case. I think that's good practice anyway; we don't support any pre-ANSI compiler and it would seem that Borland as you are running it is pre-ANSI,

I want to clarify that Borland C is as modern as it can be, and it supports everything that Clang supports -- unsurprisingly so, considering that its recent implementations are based on Clang. The one that I've just installed (Embarcadero C/C++ 7.70) is based on Clang 15, and it supports C90, C99, C11, C++98/03, C++11, C++14, C++17, and even an early draft of C++20.

This randomize() function/macro is one of the many Embarcadero-specific additions that are meant to support compatibility and workflows of earlier Borland compilers. Microsoft C also has those, and so do a bunch of Unix compilers. (Seriously, some of the functions that used to exist in earlier POSIX standards, but were later deprecated or even removed, and yet, many C compilers on Unix are still supporting those functions.)

[jbowler]

No problem; pngvalid defines feature test macros for POSIX, BSD and SVID to get functionality which is, IRC, in any one of them and pngvalid deals with the consequences. Something that defines _EMBARCADERO_SOURCE will have to deal with randomize. The requirement is that the base compiler when building libpng is STDC which means that without the feature test macros only ISO-C symbols are defined.

Now if ISO-C adds a symbol to one of the C90 headers which is not in one of the reserved sets (like the E symbols in errno.h) there would be a problem. However ISO-C has used new headers instead until last year; stdbool.h (until C23) and stdint.h (or inttypes.h).

[ctruta]

Back to this: I suggest renaming randomize to something more specific. For example, randomize_bytes. I mean, sometimes you make_random_bytes, and you get a random_byte, so (in my opinion) a name like randomize_bytes would not only fix the compilation on Borland C, but it would also make the code more readable (again -- in my opinion).

I initially had the intention to give more information in here as to "why", but now I think that PR #603 is the better place. Stay tuned!

[ctruta]

Fixed in both 'libpng16' and 'libpng18'.