midipix support / defines

[hiirotsuki]

midipix ( http://midipix.org ) is an upcoming POSIX layer for windows, builds pretty much out of box with added defined(midipix) in two spots, also compresses and decompresses correctly \o/

`diff -ur zstd/programs/bench.c zstd.new/programs/bench.c --- zstd/programs/bench.c 2016-05-07 21:45:23.964010000 +0200 +++ zstd.new/programs/bench.c 2016-05-05 16:24:24.547191263 +0200 @@ -53,7 +53,7 @@

include /* clock_t, nanosleep, clock, CLOCKS_PER_SEC */

/* sleep : posix - windows - others _/ -#if !defined(_WIN32) && (defined(unix) || defined(unix) || (defined(__APPLE) && defined(MACH))) +#if !defined(_WIN32) && (defined(unix) || defined(unix) || defined(__midipix) || (defined(APPLE) && defined(MACH)))

include

include <sys/resource.h> / setpriority /

define BMK_sleep(s) sleep(s)

@@ -70,7 +70,7 @@

define SET_HIGHPRIORITY / disabled */

endif

-#if !defined(_WIN32) && (defined(unix) || defined(unix) || (defined(__APPLE) && defined(MACH))) +#if !defined(_WIN32) && (defined(unix) || defined(unix) || defined(__midipix) || (defined(APPLE) && defined(MACH))) typedef clock_t BMK_time_t;

define BMK_initTimer(ticksPerSecond) ticksPerSecond=0

define BMK_getTime(x) x = clock()

`

[Cyan4973]

Good point @kitt3n , this is a fairly simple change, so let's support it.

[hiirotsuki]

Super! not sure why but the patch got changed from defined( midipix ) to defined(__midipix)...

[Cyan4973]

Probably related to markdown formatting. __midipix__ becomes midipix (bold character) if not enclosed in code quotes.

Also, given the project stated objective :

midipix is a development environment that lets you create programs for Windows using the standard C and POSIX APIs.

I'm surprised it doesn't work straight out of the box with just : defined(__unix__) || defined(__unix)

After all, the claim is :

If you are interested in cross-platform programming that reclaims the notion of write once, compile everywhere;

In which case, it shouldn't be necessary to modify the posix-compliant source code, not even to add a single new specific macro definition.

Do you have contacts with midipix development team ? Could they provide some hindsight to this point ?

[hiirotsuki]

Yeah, I am in #midipix on Freenode, he(midipix) hangs around there most of the time, also, he said the defines for windows should be:

#ifdef _WIN32
//win32 code
#else
//portable code
#endif

compared to how it is currently.. which would make it build out of box without define modifications for most unix-y platforms :p

[Cyan4973]

ok. It's unclear what the better patch is then : adding __midipix__, as initially suggested, or change the order in which OS Macros are tested ? The first suggestion seemed to produce less code modification.

Also, it would be good to have a way to test it, otherwise it will be a "blind" addition.

[Cyan4973]

Since adding a simple macro at a single place is a fairly benign change, I added __midipix__ as suggested to the list of posix systems.

A few caveats :

• It's untested ! we currently have no idea on how to add this target to CI environment
• I'm still puzzled why midipix doesn't simply define __unix__, which is much more common to check posix compliance.
• Even if it does not work, the worst thing that could happen is for sleep and SET_HIGH_PRIORITY to be useless, which wouldn't affect the program in any terrible way.

[midipix]

Two suggestions: one immediate, one more general, regarding the code in https://github.com/Cyan4973/zstd/blob/dev/lib/common/util.h#L66

immediate suggestion: rewrite the block, applying the pattern #if (known-non-portable-platform-A) {A-specific code} #else if (known-non-portable-platform-B) {B-specific code} # else {assume portable platform, portable code}

Using your current definitions of UTIL_sleep(), that would give you:

#  if defined(_WIN32)
#  include <windows.h>
#  define UTIL_sleep(s) Sleep(1000*s)
#  define UTIL_sleepMilli(milli) Sleep(milli)
#  define SET_HIGH_PRIORITY SetPriorityClass(GetCurrentProcess(), REALTIME_PRIORITY_CLASS)
# else
#  include <unistd.h>
#  include <sys/resource.h> /* setpriority */
#  define UTIL_sleep(s) sleep(s)
#  define UTIL_sleepMilli(milli) { struct timespec t; t.tv_sec=0; t.tv_nsec=milli*1000000ULL; nanosleep(&t, NULL); }
#  define SET_HIGH_PRIORITY setpriority(PRIO_PROCESS, 0, -20)
#  endif

--> three advantages of the above are that it 1) releases you from the need to keep track of all the portable systems out there, 2) allows your code to include newly added portable systems by default, rather than exclude them, and 3) makes supporting a newly added portable system a non-task (zero changes to your code).

• general suggestion: for as long as you support non-portable frameworks (i.e. WIN32), I'd suggest moving all of the #ifdef blocks into a designated header (say, config.h) rather than weaving them into multiple locations inside your project. That would not only keep your code proper cleaner, but also reduce the burden when you add/remove support for a non-portable platform.

[Cyan4973]

Windows isn't the only non-POSIX system out there.

POSIX cannot be "default". Default must be "standard C", which offers much less functionalities.

In utils.h, "default" (#else) just disables calls to sleep and SET_HIGH_PRIORITY. Once again, these functionalities are nice to have, but they bring nothing "essential". Disabling them is not dramatic.

[midipix]

Windows isn't the only non-POSIX system out there.

That is true, and in your project you might be supporting several of them. And yet, there are only a few non-portable systems, and since supporting them requires a special action on your part, you already know who they are. The portable systems, on the other hand, are much more of a moving target, hence my suggestion to make the portable code path the default.

In any case, seems like this should be working one way or another, so all well.

[Cyan4973]

Supported in v0.6.1

[midipix]

Excellent, thanks! Am happy to help with testing.