Hi!

10-Май-2004 12:18 [EMAIL PROTECTED] (Bart Oldeman) wrote to
[EMAIL PROTECTED]:

>> So just sent us that 10 line patch which saves those precious 5 bytes
>> in ONE mail and send the 100 line patch which just changes layout and
BO> yes! -- exactly right here. Far too much changed for so little gain.

     Again: those patch optimizes _both_ OW and BC, and for BC gain is much
more.

BO> The fact is that this patch is still much too big because it contains a
BO> mixture of many different things. incl. formatting and optimization.

     Well, I try to split this patch more.

BO> The only part of this patch that really matters is:
BO> -
BO> -        if (cntry == 0)
BO> -          cntry = (UWORD) - 1;
BO> -        else if (cntry == 0xff)
BO> /* below here is already formatting stuff -- shouldn't be in this patch */
BO> If there was another bugfix in there I missed it because it is snowed in
BO> between other changes.

     Beside _code_ (not only comments) changes, as I enumerate, there was
also fixed (enhanced, to be precisely) last parameter of DosLockUnlock.

BO> Furthermore -- I see absolutely no reason for changing
BO> -        sft FAR *p = MK_FP(r.ES, r.DI);
BO> to
BO> +        sft FAR *p = FP_PTR (sft, r.ES, r.DI);

     I see. This macro applies casting, thus, when later all uncasted
assignments will be eliminated, we may apply _also_ C++ compiler to make
more strict code checking - in C-code sometime hidden bugs, which seen by
C++ compiler.

BO> this is like changing
BO>     sft *p = malloc(sizeof *p);
BO> to
BO> #define TYPE_ALLOC(x) ((x *)malloc(sizeof x))
BO>     sft *p = TYPE_ALLOC(sft);
BO> which probably look nice to Arkady but for me malloc(sizeof *p); is much
BO> quicker to understand and more general too (if you change the type of p
BO> you don't need to change the alloc statement).

     But if you _forget_ to change argument of TYPE_ALLOC or change it
wrongly, even C compiler diagnoses this! Without TYPE_ALLOC(), if you change
type of `p', you may forget to change argument of malloc() (and often,
instead sizeof or _right_ sizeof used unrelated expressions, so called
"magic numbers"). Thus, at least, these macros are more safe.

BO> We know and understand what malloc and MK_FP do.

     Good naming may/should help. For example, yesterday you replace
expression "c >= '0' && c <= '9'" by "isnum(c)". This call is also "less
intuitive" (in your paradigm), but with good name ("isdigit" even better
name) this is even better readable.

BO> I won't change my opinion
BO> that casting pointers (or anything else really) is bad when it's not
BO> necessary. So adding fancy macros that add casts is the wrong thing to do.

     :(

BO> Similary, if you simply split
BO> UWORD callerARG1;
BO> into
BO> UBYTE callerARG0, callerARG1;
BO> you can get cleaner code without fancy lobyte/hibyte macros or ugly casts.

     Ok, I do this (replace UWORD by `xreg').

BO> And for
BO> const psp _seg *p = FP_SEG_PTR (const psp, cu_psp);
BO> why not simply use
BO> const psp _seg *p = MK_SP(cu_psp);
BO> where MK_SP works like MK_FP but returns a (void _seg *). That looks a lot
BO> less intrusive to me.

     Two reasons: first, casting (above I show reasons why this is
important), second, because "MK_SP" at first glance seen as "MK_FP" (thus,
easy to make mistake when retype similar code). Of course, second reason is
mostly question of taste, and I will not object against other names (like
MK_SEG_PTR or even MK_SEG). Consequently, FP_PTR may be named MK_PTR.

BO> Anyway -- I was playing with _seg pointers myself indeed but then stopped
BO> because it only helps Borland compilers. Sure the kernel needs to

     BTW, OW also presents "__based" and "__segment" keywords.
Unfortunately, they are not very comparable to _seg. :(

BO> *compile* with Borland, but it only needs to be optimized with Open
BO> Watcom. Introducing _seg pointers is OK (after all you say "this
BO> pointer is not a general far pointer but one with offset 0" which gives
BO> more information) but not so important, as it is purely a matter of style.




-------------------------------------------------------
This SF.Net email is sponsored by Sleepycat Software
Learn developer strategies Cisco, Motorola, Ericsson & Lucent use to deliver
higher performing products faster, at low TCO.
http://www.sleepycat.com/telcomwpreg.php?From=osdnemail3
_______________________________________________
Freedos-kernel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/freedos-kernel

Reply via email to