FDOS / freecom

FreeDOS Command Shell (command.com)
http://www.freedos.org/
GNU General Public License v2.0
155 stars 38 forks source link

Build is broken for ia16 gcc #79

Closed andrewbird closed 2 years ago

andrewbird commented 2 years ago

I presume this is because @tkchia's libi86 now has support for fmem functions. I'm not sure what the correct fix is here though.

ia16-elf-gcc -c -I.. cntry.c @gcc.cfg -o cntry.obj
In file included from cntry.c:115:0:
../fmemory.h:51:6: error: conflicting types for ‘_fmemcpy’
 void _fmemcpy(void far * const dst, const void far * const src, unsigned length);
      ^~~~~~~~
In file included from /usr/ia16-elf/include/libi86/internal/wrap/string.h:7:0,
                 from cntry.c:111:
/usr/ia16-elf/include/libi86/string.h:41:23: note: previous declaration of ‘_fmemcpy’ was here
 extern __libi86_fpv_t _fmemcpy (__libi86_fpv_t __dest, __libi86_fpcv_t __src,
                       ^~~~~~~~
In file included from cntry.c:115:0:
../fmemory.h:52:6: error: conflicting types for ‘_fmemset’
 void _fmemset(void far * const dst, int ch, unsigned length);
      ^~~~~~~~
In file included from /usr/ia16-elf/include/libi86/internal/wrap/string.h:7:0,
                 from cntry.c:111:
/usr/ia16-elf/include/libi86/string.h:47:23: note: previous declaration of ‘_fmemset’ was here
 extern __libi86_fpv_t _fmemset (__libi86_fpv_t __dest, int __c,
                       ^~~~~~~~
In file included from cntry.c:115:0:
../fmemory.h:58:6: error: conflicting types for ‘_fmemmove’
 void _fmemmove(void far * const dst, const void far * const src, unsigned length);
      ^~~~~~~~~
In file included from /usr/ia16-elf/include/libi86/internal/wrap/string.h:7:0,
                 from cntry.c:111:
/usr/ia16-elf/include/libi86/string.h:43:23: note: previous declaration of ‘_fmemmove’ was here
 extern __libi86_fpv_t _fmemmove (__libi86_fpv_t __dest, __libi86_fpcv_t __src,
                       ^~~~~~~~~
make: *** [../../mkfiles/gcc.mak:30: cntry.obj] Error 1
tkchia commented 2 years ago

Hello @andrewbird,

The main difference seems to be that my libi86 functions return a pointer to the destination buffer in each case — I follow Open Watcom's declarations for these functions.

Perhaps you can try adding return values to FreeCOM's versions of these functions as well — both in the declarations and definitions — so that the prototypes will agree with those from libi86. This should hopefully not be too hard.

Thank you!

andrewbird commented 2 years ago

Hello @tkchia, That's good to know, but I'm working on a label problem at the moment, so if anyone else wants to jump in and fix it it would be most welcome.

Thank you!

tkchia commented 2 years ago

Hello all,

Here is my proposed patch:

diff --git a/suppl/fmemory.h b/suppl/fmemory.h
index 079509f..1c24b0f 100644
--- a/suppl/fmemory.h
+++ b/suppl/fmemory.h
@@ -48,14 +48,14 @@
 #define _fmemset farmemset

 #else  /* !_PAC_NOCLIB_ */
-void _fmemcpy(void far * const dst, const void far * const src, unsigned length);
-void _fmemset(void far * const dst, int ch, unsigned length);
+void far *_fmemcpy(void far * const dst, const void far * const src, unsigned length);
+void far *_fmemset(void far * const dst, int ch, unsigned length);
 #endif /* _PAC_NOCLIB_ */

 unsigned _fstrlen(const char far * const s);
 char far *_fstrchr(const char far * const s, int ch);
 char far *_fmemchr(const char far * const s, int ch, unsigned length);
-void _fmemmove(void far * const dst, const void far * const src, unsigned length);
+void far *_fmemmove(void far * const dst, const void far * const src, unsigned length);
 int _fmemcmp(const void far * const dst, const void far * const src, unsigned length);
 int _fmemicmp(const void far * const dst, const void far * const src, unsigned length);
 int _fstrcmp(const char far * const dst, const char far * const src);
diff --git a/suppl/src/fmemcpy.c b/suppl/src/fmemcpy.c
index 8e38d72..663c47f 100644
--- a/suppl/src/fmemcpy.c
+++ b/suppl/src/fmemcpy.c
@@ -58,7 +58,7 @@ void _fmemcpy(unsigned const dseg, unsigned const dofs
 #include <portable.h>
 #include "fmemory.h"

-void _fmemcpy(void far * const s1, const void far * const s2, unsigned length)
+void far *_fmemcpy(void far * const s1, const void far * const s2, unsigned length)
 {  byte far*p;
    const byte far*q;

@@ -68,6 +68,7 @@ void _fmemcpy(void far * const s1, const void far * const s2, unsigned length)
        do *p++ = *q++;
        while(--length);
    }
+   return s1;
 }

 #endif
diff --git a/suppl/src/fmemove.c b/suppl/src/fmemove.c
index 02022f7..8706dd5 100644
--- a/suppl/src/fmemove.c
+++ b/suppl/src/fmemove.c
@@ -83,14 +83,14 @@ void _fmemmove(unsigned dseg, unsigned dofs
 #include <portable.h>
 #include "fmemory.h"

-void _fmemmove(void far * const s1, const void far * const s2
+void far *_fmemmove(void far * const s1, const void far * const s2
    , unsigned length)
 {  byte far *p;
    byte far *q;
    byte far *h;

    if(!length)
-       return;
+       return s1;

    p = _fnormalize(s1);
    q = _fnormalize((void far*)s2);
@@ -117,7 +117,7 @@ void _fmemmove(void far * const s1, const void far * const s2
                --> copy backwardly
    */
    if(p == q)
-       return;
+       return s1;

    /*
     * without the typecasts TC++1 ignores the segment portions completely
@@ -136,6 +136,7 @@ void _fmemmove(void far * const s1, const void far * const s2
    else {
        _fmemcpy(s1, s2, length);
    }
+   return s1;
 }
 #endif
 #endif

Thank you!

andrewbird commented 2 years ago

Hello @tkchia, Thanks for this, the build completes now for me successfully. Just one question, if libi86 has watcom compatible fmem* functions, why didn't its compilation fail?

Thanks again!

tkchia commented 2 years ago

Hello @andrewbird,

Just one question, if libi86 has watcom compatible fmem* functions, why didn't its compilation fail?

The functions were only available through <libi86/string.h> before, not <string.h>.

In newer versions (~ June 2022) of gcc-ia16 + newlib-ia16 + libi86, I added a compiler mechanism to allow <string.h> to also automatically include <libi86/string.h> (unless in __STRICT_ANSI__ mode).

Thank you!

andrewbird commented 2 years ago

Hello @tkchia , sorry I didn't explain very well. I meant why didn't watcom's compilation fail? Thank you!

tkchia commented 2 years ago

Hello @andrewbird,

Oh. Apparently the problematic declarations were only included if

#if defined(_PAC_NOCLIB_) || defined(_TC_EARLY_) || defined(__GNUC__)

so the conflicting declarations and definitions were not included when using Open Watcom.

(I suppose, if you want to, you could also try to distinguish between "early" and "later" versions of gcc-ia16 in suppl/supl_def.h, suppl/src/fmemove.c, etc. Something like

#ifdef __GNUC__
#if __ia16__ - 0 < 20220606L
#define _GCC_EARLY_
#else
#define _GCC_LATER_
#endif
#endif

etc. might work, though I have not tested this.)

Thank you!

andrewbird commented 2 years ago

Hello @tkchia, I tried a quick variation on your latest suggestion, and I deleted the || defined(__GNUC__) in various places. The main reason for trying this was to see if the resultant command.com was smaller if built using your _fmem*() functions. However, it seems the supplement library is still needed for some functions like _fstrcpy() which I don't believe your libi86 implements yet. So from my point of view I think your first patch is good to be applied, and perhaps later if things change we can revisit. How about creating a PR?

Many thanks!