Supplement: Match fmem* functions to common prototypes

[andrewbird]

Since ia16-elf-gcc's libi86 received support for fmemcpy, fmemmove and fmemset, its prototypes are in conflict with the replacement functions defined in the suppl directory. If libi86 implements all of the support required for FreeCOM in the future, then we may not need to include suppl, but until then adjust the suppl to match the common prototypes.

Patch by @tkchia, and discussion at [fixes #79]

[andrewbird]

Hello @tkchia, I see you've added fstrcpy()which allowed me to get further in my quest not to use the supplement functions in FreeCOM, but now I'm seeing another missing function.

is_fnamc.c: In function ‘is_fnchar’:
is_fnamc.c:60:7: error: implicit declaration of function ‘_fmemchr’ [-Werror=implicit-function-declaration]
    || _fmemchr(nlsBuf->illegalChars, c, nlsBuf->illegalLen));
       ^~~~~~~~
cc1: all warnings being treated as errors
make: *** [../mkfiles/gcc.mak:30: is_fnamc.obj] Error 1

I don't want to keep pestering you about these, as currently the compilation is fine, but would you like me to keep reporting them?

Many thanks!

[tkchia]

Hello @andrewbird,

Thanks for the report. Are you able to come up with a more complete list of the missing functions?

Thank you!

[andrewbird]

Hello @tkchia, Yes I agree it's not good to keep dripping these reports one per function. I'll try to do a review of the fmem* functions that the supplement file provides and see which are actually in use.

Thank you!

[andrewbird]

So looking at the definitions of the supplement I can see the following fmem functions are defined:

• [X] USED void far _fmemcpy(void far const dst, const void far * const src, unsigned length);
• [X] USED void far _fmemset(void far const dst, int ch, unsigned length);
• [X] USED unsigned _fstrlen(const char far * const s);
• [ ] UNUSED char far _fstrchr(const char far const s, int ch);
• [X] USED char far _fmemchr(const char far const s, int ch, unsigned length);
• [X] USED void far _fmemmove(void far const dst, const void far * const src, unsigned length);
• [X] USED int _fmemcmp(const void far const dst, const void far const src, unsigned length);
• [ ] UNUSED int _fmemicmp(const void far const dst, const void far const src, unsigned length);
• [ ] UNUSED int _fstrcmp(const char far const dst, const char far const src);
• [ ] UNUSED int _fstricmp(const char far const dst, const char far const src);
• [X] USED void _fstrcpy(char far const dst, const char far const src);

I've then run git grep for each of them, and marked them as USED wherever I can see a use that might be used outside of the suppl directory, or within another function in the suppl directory. I've also added the checkbox to show which you already implement. So with the support you've just added, it looks like it should cover freecom build. I'll let you know as soon as the latest gets into the Ubuntu PPA.

[tkchia]

Hello @andrewbird,

Thanks! Updated libi86-ia16-elf packages should be available soon (perhaps in an hour or so — once Launchpad publishes them). I will also look into possibly implementing the other functions that suppl covers.

Thank you!

[andrewbird]

Hello @tkchia, I have the updated packages now, and can confirm that freecom builds to completion with my local patch. I do notice that command,com grew slightly (100bytes) but it's still smaller than the Watcom version.

I will also look into possibly implementing the other functions that suppl covers.

I think there must be some more prototype mismatches as the functions you recently added now interfere with the ones in suppl without my local patch, so you might find further ones as you add the missing functions.

I'm not quite ready to submit my patch to freecom. I have to figure out what load_icd is, when it's used and whether it makes sense as a DOS executable in the cross compiled environment.

Many thanks!

[andrewbird]

Hello @tkchia, I added a topic branch https://github.com/andrewbird/freecom/tree/t2 with my current changes. I didn't bother with the support for early gcc, it's free software available to all and I don't see any reason why anyone would want to run an older version.

Many thanks!

[andrewbird]

Hello @tkchia, I have one last remaining use of one of your new functions that I don't seem to be able to get rid of:

diff --git a/suppl/src/nlstime.c b/suppl/src/nlstime.c
index 1a97fb8..cb8d781 100644
--- a/suppl/src/nlstime.c
+++ b/suppl/src/nlstime.c
@@ -55,7 +55,7 @@ static char const rcsid[] =
        "$Id$";
 #endif

-#if defined(_MICROC_) || defined(_TC_EARLY_) || defined(__GNUC__)
+#if defined(_MICROC_) || defined(_TC_EARLY_)
 #undef HAVE_DOSTIME
 #else
 #define HAVE_DOSTIME

nlstime.c: In function ‘nlsCurTime’:
nlstime.c:67:19: error: storage size of ‘dt’ isn’t known
  struct dostime_t dt;
                   ^~
nlstime.c:76:2: error: implicit declaration of function ‘_dos_gettime’ [-Werror=implicit-function-declaration]
  _dos_gettime(&dt);
  ^~~~~~~~~~~~
nlstime.c:67:19: error: unused variable ‘dt’ [-Werror=unused-variable]
  struct dostime_t dt;
                   ^~
cc1: all warnings being treated as errors
make: *** [../../mkfiles/gcc.mak:31: nlstime.obj] Error 1

This comes about because there's a file called ./suppl/compat/dos.h that shadows the compiler's one. If I just try to rename it as a test, I see more prototype mismatches.

ia16-elf-gcc -c -I.. addu.c @gcc.cfg -o addu.obj
In file included from ../portable.h:137:0,
                 from addu.c:39:
../p-gcc.h:115:19: error: conflicting types for ‘_dos_getftime’
 static inline int _dos_getftime(int fd, unsigned *date, unsigned *time)
                   ^~~~~~~~~~~~~
In file included from ../regproto.h:29:0,
                 from ../portable.h:29,
                 from addu.c:39:
/usr/ia16-elf/include/dos.h:190:17: note: previous declaration of ‘_dos_getftime’ was here
 extern unsigned _dos_getftime (int __handle,
                 ^~~~~~~~~~~~~
In file included from ../portable.h:137:0,
                 from addu.c:39:
../p-gcc.h:125:19: error: conflicting types for ‘_dos_setftime’
 static inline int _dos_setftime(int fd, unsigned date, unsigned time)
                   ^~~~~~~~~~~~~
In file included from ../regproto.h:29:0,
                 from ../portable.h:29,
                 from addu.c:39:
/usr/ia16-elf/include/dos.h:202:17: note: previous declaration of ‘_dos_setftime’ was here
 extern unsigned _dos_setftime (int __handle, unsigned __date, unsigned __time);
                 ^~~~~~~~~~~~~
In file included from ../portable.h:137:0,
                 from addu.c:39:
../p-gcc.h:135:25: error: conflicting types for ‘_dos_getvect’
 static inline void far *_dos_getvect(int intno)
                         ^~~~~~~~~~~~
In file included from ../regproto.h:29:0,
                 from ../portable.h:29,
                 from addu.c:39:
/usr/ia16-elf/include/dos.h:243:23: note: previous declaration of ‘_dos_getvect’ was here
 extern __libi86_isr_t _dos_getvect (unsigned __intr_no);
                       ^~~~~~~~~~~~
In file included from ../portable.h:137:0,
                 from addu.c:39:
../p-gcc.h:144:20: error: conflicting types for ‘_dos_setvect’
 static inline void _dos_setvect(int intno, void far *vect)
                    ^~~~~~~~~~~~
In file included from ../regproto.h:29:0,
                 from ../portable.h:29,
                 from addu.c:39:
/usr/ia16-elf/include/dos.h:244:13: note: previous declaration of ‘_dos_setvect’ was here
 extern void _dos_setvect (unsigned __intr_no, __libi86_isr_t __isr);
             ^~~~~~~~~~~~
make: *** [../../mkfiles/gcc.mak:31: addu.obj] Error 1

I'm not really sure what the best path forward is?

Thank you!

[andrewbird]

Hello @tkchia, I removed the offending file suppl/compat/dos.h, and commented some of the replacement functions which seemed to be GCC specific.The compilation now succeeds, and the filesize is now back down equal to the original size.

Thank you!

[tkchia]

Hello @andrewbird,

Most of the conflicts in the prototypes are relatively minor:

• an unsigned return value vs. an int (this is for the MS-DOS error codes), and

• void __far * vs. void __interrupt __far (*) () for interrupt handler pointers.

  But there is one prototype conflict that needs special attention. The definition of the intr function in suppl uses the stdcall parameter passing convention

  extern void intr(int nr, union REGPACK *r) __attribute__((stdcall));

  and the function itself is defined in suppl/src/intr.asm. libi86 on the other hand uses whatever calling convention is set on the compiler command line (e.g. regparmcall):

  extern void intr (int, union REGPACK *);

  This means, mixing libi86 's declaration for intr with suppl 's definition will lead to incorrect code. So if you decide to wholly remove suppl/compat/dos.h, you will also need to remove suppl/src/intr.asm from the final link.

---

(If I remember correctly, Bart Oldeman came up with the __attribute__((stdcall)) hack in suppl, in order to simplify the code in suppl/src/intr.asm. This was back before my libi86 was even a thing.)

Thank you!

[tkchia]

Hello @andrewbird,

OK... I just remembered that the situation with intr was in fact a little bit messier than what I said.

Bart Oldeman wrote a whole implementation of intr in suppl/src/intr.asm   even for Open Watcom, which already implemented this function. At that time we needed the intr function to load flags from the union REGPACK before issuing the int opcode (to work around a DOSBox issue), and OW's intr did not guarantee that. (See https://github.com/FDOS/freecom/pull/7 and https://www.mail-archive.com/freedos-devel@lists.sourceforge.net/msg11641.html .)

I am not sure how much FreeCOM still relies on this particular behaviour. Anyway, nowadays libi86 includes an _intrf function, and OW an intrf function. Both of these functions are guarantee to load flags beforehand.

Maybe a good course of action for now is to

• rename the intr function in suppl/ to _intrf or intrf (or both), and
• make its prototype agree with the corresponding functions in both the gcc-ia16 + libi86 and Open Watcom toolchains.

Let me see if I can whip up a quick PR.

Thank you!

[andrewbird]

Hello @tkchia, Just in case you didn't notice this also made the resultant command.com smaller(132 bytes)!

Thank you!

[tkchia]

@andrewbird : that is really nice to hear. Thank you!