Hi,
the following is more general and is not only related to the small
patch:
On Fri, 10 Dec 1999 22:02:38 +0100 (MET), Asger K. Alstrup Nielsen
wrote:
>> >> +/** AHanses: Eliminate unneeded parameter for Unix: 'fork(void)'
>> >> + Unix will just ignore the spawnFlag parameter
>> >> +*/
>> >
>> >This comment should go. The remark that Unix should ignore the
>> >spawnFlag should go as a comment near the Fork-method.
>>
>> No problem. This was mostly for me. Nevertheless one personal note
>> here:
>> When reading sources originally written for OS/2 and compare them with
>> the usual hacks from bsd or GNU I see one important difference:
>>
>> The comments / code ratio is normally 2 / 1 or even 3 / 1 with broad
>> coverage of design, implementation, security, preconditions,
>> postconditions, restrictions, etc. It seems the usual Unix hacker
>> thinks he does not need to extensively comment out the source to avoid
>> design and implementation bugs. Why?
>
>The comment you wrote is placed the wrong place.
Yes, of course this a nice example how not to do comment. :)
>Also, please read the SystemCalls header file in cvs. It's richly
>commented.
No doubt. I was not aiming at this one. Other headers and implementions
are not so well commented (I'm currently thinking of figinst.h/C). My
points are:
1) In open source with a large number of rapidly changing contributors
comments are crucial to attract new collaborators and to assure project
survival when core contributors change. I think everybody knows
examples of failures.
2) LyX has the specific problem of complexity: Contributors like me,
who will never become really skilled programmers (but who may
contribute a huge number of small bits and pieces which, summed up, are
crucial) are likely to become frightened. So comments should become a
kind of short tutorial. And:
3) People tend never to read abstract documents. They read sources. So
there you should find the actual design documents. And doc++ is a good
thing to distill them.
So my idea was something similar to this:
Setup each source file with a comment form just at the beginning,
something like the following and ask developers to fill them by and by
(I think this will work better than abstract recommendations):
/*** xy.C: For usage and design info please refer to xy.h. Please add
here only comments describing the specific implementation. Those
comments should ease maintenance and bug tracing. So please be careful.
Please add a test of your implementation as a comment at the end of
this module.
**/
/*** xy.h: THE XY INTERFACES AND HOW TO USE THEM
SYNOPSIS and DESIGN PRINCIPLES:
DESCRIPTION OF WHAT THEY SHOULD DO:
PRECONDITIONS AND ASSUMPTIONS:
POSTCONDITIONS AND LIMITATIONS:
>
>> >Finally, the patch should be remade against the latest cvs version.
>>
>> I think I better let you do this, otherwise I might introduce more
>> bugs.
>
>If you do it, I'll review the patch before we apply it. I don't have
>time to redo the patch, and I can't possibly test it on OS/2.
Another somewhat more general point here: cvs is rapidly changing. Once
having setup some working patch the update is likely to break it. When
I've understood 50% of what's going on in the new version and have even
managed to understand the cvs manual there will have appeared the next
revision breaking everything again. So all in all it might be simpler
and faster for everybody if the core developer who is responsible for
this branch and doing most of the changes is somehow 'patronizing' a
patch, so as not to break it, etc. Or to wait for a 'frozen' design.
>
>> Please note that signals often are a portability headache, especially
>> when sending them to other processes. Did you investigate how this
>> works, e.g. in cygwin?
>
>No I did not. As with all other system code in LyX: We write it first,
>and then we learn about the problems with it when it is deployed on
>different platforms. This is the most cost effective way to go, and
>is the best way to take advantage of the open source development model
>where we have many testers.
Is it really? Bug tracing is AFAIK all in all by far the most costly
part of software development (some 10:1 or 20:1 depending on
complexity). Some kind of 'quality management' might be painful at
first but most cost effective on the long run.
>> >Would it be possible to use the alternative that waitForChild is
>> >different on EMX?
>>
>> Possible, yes! But why?
>
>Because waitForChild might be used several places, and then we will
>only need one #ifdef guard.
>
>> >> // Wait for child process to finish. Returns returncode from child.
>> >> +#ifndef __EMX__
>> >> void Systemcalls::waitForChild()
>> >> {
>> >
>> >> }
>> >> +#endif
>> >
>> >See above: Instead of dropping this function, please just make
>> >the waitForChild operation a dummy operation.
>>
>> No problem, but why bloat classes with dummy members? (IMHO the LyX
>> classes are bloated, I guess you really need some 60% of the puublic
>> members)
>
>The method is not dummy on Unix. It's better to have the same structure
>across platforms, even if the implementation is different (or missing.)
>
If a child is started with exec*() instead of spawn*() emx might need a
waitForChild() which is not a dummy. Making it dummy is a good way to
introduce a hard to trace bug in the future.
Because of past problems like this when faking an absent functionality
I
generally prefer an unresolved reference to a potentially buggy dummy,
so that I can check/rewrite the relevant code (e.g. using spawnvp()) in
place, if need be. Or I might use a dummy then, if I'm sure you'll
never use exec*() anymore.
So I think: Better let the linker refuse linkage than let a buggy app
send your work to nirwana.
>> >> - const int MAX_ARGV = 255;
>> >
>> >If the problem is that MAX_ARGV might be defined already in OS/2,
>> >I think the better solution is to rename this local constant.
>>
>> I think it might be better to catch the OS' default value.
>
>It's still possible:
>
>#ifdef MAX_ARGV
>int const max_argv = MAX_ARGV;
>#else
>int const max_argv = 255;
>#endif
Rechecked this, now I cannot remember why the heck I wrote this ;-)
(Perhaps confusing something?) So much about missing comments...
But you proposal is still nice, I think, because now we can move
MAX_ARGV to config.h.
I might even dare to propose:
Move **all** parameters to 'config.h', which introduce somewhat
arbitrary limits and may cause bugs in unexpected stress conditions.
E.g. in this case when switching to variable argument vectors instead
of using the shell you might soon exceed such a limit. Just think of
those awfully long paths that Unix sys admins seem to love....
Sorry, if this sounds negative, it's meant to be only positive.
Greets,
Arnd
