Siegbert Debatin wrote:
Georg Mischler wrote:
I didn't see any specfic fixes from you. Did you only intend
to send those, or is this a mail system failure?I sent them on Tuesday morning this week to [email protected]. I just
redirected the original mail to you. Please give me a note, if you didn't
receive it.
Mail with a *.zip attachment is sorted into a seperate folder
on my server, together with all the viruses and worms. Quite
likely that I didn't spot yours in between those and accidentally
deleted it.
But I got the second copy, and applied as much of it as I could.
The changes are checked in and should be in tomorrows HEAD dump.
Following below are my comments to those items where I didn't
simply use your fix. The explanations and questions may also be
interesting for others to better understand the task at hand.
If anyone sees a specific problem that they'd like to tackle,
please speak up so we can discuss the details.
1.
You added a "typedef RT_PID pid_t" to common/rtprocess.h.
However, we shouldn't be using pid_t, as it is platform specific.
I found only one place where it was still used, in the dummy
definition of kill() in util/win_netproc.h. I removed this
function, because the new win_kill() in common/win_process.c
should be used instead (but see next item).
2.
The kill(pid, SIGALRM) in gen/mkillum.c was added recently and
points to a more general problem on Windows. First there is no
kill() in Windows anyway. I added a #define in rtprocess.h, so
that it will call win_kill() and ultimately SafeTerminateProcess()
from common/win_process.c. But this only solves part of the
problem, because it ignores the signal number and always does the
equivalent of a "kill -9". The reason unix uses SIGALRM here is
to alert the subprocess, so that it has a chance to clean up its
temporary files etc. before exiting. I assume there's equivalent
notification functionality available in the native Win32 APIs.
Maybe we should create an abstraction like "send_alarm_to_die()"
to generalize the task?
3.
This leaves us with select() not linked in gen/mkillum2.c.
The select() call was recently added for parallel processing.
It may be possible to compile this with the right libaries, but
it will NOT work on Windows because the Windows select() only
supports sockets. Does anyone know a good alternative to poll a
number of file descriptors without doing a busy-wait on Windows?
4.
What does Windows define in sys/file.h? Is it present on all
versions? In that case, it might be better to include that in
common/paths.h or common/platform.h instead of directly in
ot/obj2mesh.c.
You also added sys/file.h to rt/rpict.c, but in a section that is
only relevant to posix-but-non-bsd systems, which I assume is not
what you meant. Did moving platform.h further up there solve
anything?
5.
The missing goto label in util/rad.c was still present in the
sources to Radiance3.3 (from DR), so I added it again.
That code allows to pass view parameters in through environment
variables. This is/was necessary for programs that are called
through system()/popen(), where the shell would limit the number
of arguments passed to a subprocess. I'm not sure if it is still
needed, or if it even might be helpful on other platforms as
well.
6.
including sys/wait.h only for POSIX systems in util/rpiece.c is
ugly, but the simplest workaround for the moment. We'll need an
abstraction for the process management in rpiece to make this
portable.
7.
You changed the first argument of bfree() in common/bmalloc.c to
char*, so we can operate arithmetics on it (analog to my fix for
bposition). However, changing the signature of that function
would require numerous casts elsewhere. I now created a seperate
local char* variable, which should give us the positive effect
without the signature change.
Actually, what was the latest consensus again? Do we really still
need common/bmalloc.c anymore on any current platforms?
8.
You included resolu.h a second time in gen/genrev.c. Did that
change anything, or was the actual solution to link against
rtio.lib?
9.
Ra_bmp.exe now links against rtproc.lib, and common/tmapcolrs.c
includes rtprocess.h, so that they will have our popen()
replacement available and should build on Windows as well.
-schorsch
···
--
Georg Mischler -- simulations developer -- schorsch at schorsch com
+schorsch.com+ -- lighting design tools -- http://www.schorsch.com/