Hi!
11-Июн-2006 20:12 [EMAIL PROTECTED] (Blair Campbell) wrote to
[email protected]:
>> BC> +++ goxy.c 11 Jun 2006 02:47:05 -0000 1.3
>> BC> - regs.h.dh = y - 1;
>> BC> - regs.h.dl = x - 1;
>> BC> + regs.r_dx = ( ( y - 1 ) & 0xFF ) | ( ( x - 1 ) >> 8 );
>> Bug! Should be replaced by
>> regs.r_dx = (((y-1) & 0xFF) << 8) | ((x-1) & 0xFF);
BC> Note that this is surrounded by #if 0... (it's not even used )
But later someone reverts back to this code (for example, for porting
to OW), and he get wrong code.
>> BC> + _AH = 0x02;
>> This adds more incompatibility and prevents (future) porting to OW.
>> :(
BC> So? whoever did that would probably have to heavily modify OW's clib
BC> anyways...
And this is one of the FreeCOM problems! And why _add_ more
incompatibilities, not eliminate them?!
>> BC> +++ mk_rddir.c 11 Jun 2006 02:47:05 -0000 1.5
>> BC> + r.r_flags = 1;
>> BC> + intr( 0x21, &r );
>> Again bug: you have no ways to pass _your_ Carry flag value into
>> interrupt through int*() functions.
BC> But it works.
Remove "r.r_flags = 1;" or replace it by "r.r_flags = 0;" - and you get
precisely same results.
And, if you not trust me - study intr() sources (in INTR.CAS file in
RTL sources). You will see this yourself.
>> BC> +#define mkdir(x) lfn_mrc_dir(x,0x39)
>> (2) Why not use explicitly 0x7139 and thus simplify (1)?
BC> Because it works.
:) Of course. But I say "simplify".
>> BC> +++ cd_dir.c 11 Jun 2006 02:47:05 -0000 1.7
>> BC> +int _Cdecl lfn_mrc_dir( const char *name, int function );
>> Bug! In mk_rddir.c there is not same prototype!
BC> Since _Cdecl is automatic in Turbo C, it _IS_ the same prototype.
Later, if by some reason you add -p option (to change calling
convention) or when you port this code to OW, then you get non-working code.
BC> And ESPECIALLY (please) do not call things bugs that don't even warn
BC> and that work perfectly.
If some bug is currently _masked_ (latent), then it anyway remains bug.
>> BC> +++ farread.c 11 Jun 2006 02:47:05 -0000 1.3
>> BC> +unsigned DOSreadwrite(int fd, void far *buffer, unsigned size,
>> BC> + unsigned short func );
>> Prototypes should be present _in one place_ (somewhere in headers)!
BC> Well, the prototype is present, and since it's only used in two
BC> places, it hardly warrants being put in a header.
It _always_ warranted! Especially, if tomorrow you (or someone else)
will change original function and forget, that there is another prototype in
other place...
>> BC> +++ cmdinput.c 11 Jun 2006 02:47:05 -0000 1.6
>> BC> static void outc(char c)
>> Wow! Why in so many places use write(), whereas there is defined
>> outc()?!
BC> It happens to be static
Who prevents you from removing this modifier? :)
PS: Because `static' here used to indicate, that function is "local", I
prefer to introduce this macro:
#ifdef DEBUG
# define LOCAL
#else
# define LOCAL static
#endif
...
LOCAL void outc (char c) ...
_______________________________________________
Freedos-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/freedos-devel
