Project

General

Profile

Bug #7332

pointer cast to integer in make code

Added by Georg Sauthoff over 3 years ago. Updated over 3 years ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
-
Start date:
2016-08-28
Due date:
% Done:

0%

Estimated time:
Difficulty:
Bite-size
Tags:
needs-triage
Gerrit CR:

Description

I took the illumos make source code and ported it such that it is also usable on other operating systems (e.g. Linux):

https://github.com/gsauthof/somake

Some of the changes are perhaps also suitable for the upstream.

Most notably, I removed some pointer<->int casting, which is undefined behavior and likely breaks for 64 bit builds:

https://github.com/gsauthof/somake/commit/43a5295e69964a5c6c9935656e8ac2e3febe1d8b#diff-86b4d73feedf1eaca4632f03b3392023L112

Also part of this change set are some additional includes of standard headers - that also shouldn't hurt upstream.

Another less serious smaller change to fix 64 bit builds on Solaris 10:

https://github.com/gsauthof/somake/commit/1755c692964151be62c191089bb228528b807e0a

Apparently, sys_nerr was removed from the 64 bit Solaris environment.

The Linux linker also issues a deprecation warning for sys_nerr. Probably all places where sys_nerr is used can be easily refactored to just use strerror() because calling strerror() with a non-system errno value assumingly yields result similar to the custom printing.

On Linux, there is also a linker warning generated for `mktemp`:

CMakeFiles/somake.dir/bin/parallel.cc.o: In function `execute_parallel(_Property*, Boolean, Boolean)':
parallel.cc:(.text+0x1811): warning: the use of `mktemp' is dangerous, better use `mkstemp'

History

#1

Updated by Gary Mills over 3 years ago

Comments introduced with `//' are only valid in C++ or C99 code. Otherwise, you should use `/* ... */'.

Some of the functions defined explicitly in the code are actually available in header files on Solaris. For example, getwd() is defined in the unistd.h header. There's no need to define it explicitly in the code. The man page for getwd, for example, gives you the header file.

#2

Updated by Georg Sauthoff over 3 years ago

Gary Mills wrote:

Comments introduced with `//' are only valid in C++ or C99 code. Otherwise, you should use `/* ... */'.

I don't target pre-C99 compilers that don't support `//`.

Some of the functions defined explicitly in the code are actually available in header files on Solaris. For example, getwd() is defined in the unistd.h header. There's no need to define it explicitly in the code. The man page for getwd, for example, gives you the header file.

Just to be clear, the redundant `getwd()` declaration including some CPP logic was imported from upstream:

/* On linux getwd(char *) is defined in 'unistd.h' */
#ifdef __cplusplus
extern "C" {
#endif
extern char *getwd(char *);
#ifdef __cplusplus
}
#endif

https://github.com/illumos/illumos-gate/blob/67c3092ccd4e8c261df7eded9df072ff9c4e330b/usr/src/cmd/make/include/mk/defs.h#L352-L358

I just wrapped that as Solaris specific because it generates compile errors on other platforms (due to deprecation attributes being present in the `unistd.h` decaration).

Due to the marking of `getwd()` as LEGACY in POSIX.2001 and the fact that it the declaration is present in the original code I didn't bother to look it up in a Solaris man page.

Funnily, `getwd()` isn't even used in any translation unit of the make project.

Thus, to simplify things it makes sense to remove that declaration in defs.h, upstream and in the port.

Also available in: Atom PDF