Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ctruta opened this issue Sep 22, 2024 · 7 comments
Closed

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

ctruta opened this issue Sep 22, 2024 · 7 comments

Comments

@ctruta
Copy link
Member

ctruta commented Sep 22, 2024

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

randomize(void *pv, size_t size)

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
Copy link
Contributor

jbowler commented Sep 22, 2024

Yes, random is expected to be defined in <stdlib.h> 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
Copy link
Contributor

jbowler commented Sep 22, 2024

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
Copy link
Contributor

jbowler commented Sep 22, 2024

PR #603 you need PR #602 to test.

@ctruta
Copy link
Member Author

ctruta commented Sep 23, 2024

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
Copy link
Contributor

jbowler commented Sep 23, 2024

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
Copy link
Member Author

ctruta commented Oct 18, 2024

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
Copy link
Member Author

ctruta commented Oct 18, 2024

Fixed in both 'libpng16' and 'libpng18'.

@ctruta ctruta closed this as completed Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants