npat-efault / picocom

Minimal dumb-terminal emulation program
GNU General Public License v2.0
631 stars 125 forks source link

Musl build failure with" has no member named 'c_ispeed'" #100

Closed rc-matthew-l-weber closed 6 years ago

rc-matthew-l-weber commented 6 years ago

Got a similar failure on a buildroot x86_64/i386 picocom 3.1 build. I still need to dig into the config but the build end log has the build output of the matching failure. These look like musl failures. http://autobuild.buildroot.net/results/3d874d1bc838f69b6411d018065015f851caf1e9 http://autobuild.buildroot.net/results/9cd/9cd07e1d2518c5745269d28cac07a6a4937c4c79//

baruchsiach commented 6 years ago

musl libc does not provide c_ispeed and c_ospeed fields in struct termios. I posted a patch to Buildroot that disables custom baud rate when using musl libc.

http://patchwork.ozlabs.org/patch/896568/

rc-matthew-l-weber commented 6 years ago

Thanks Baruch. From a package level, should I close this issue or leave it as MUSL support compatibility is a desired feature?

baruchsiach commented 6 years ago

This issue is real. The default build fails with musl for x86 targets. My Buildroot patch is just a workaround for the build failure, not a real fix.

npat-efault commented 6 years ago

Yes this is a real issue. We should modify picocom to not enable custom baud support when building with musl, by editing custbaud.h. I will look into it, but a PR would be appreciated.

npat-efault commented 6 years ago

Also if someone could leave a comment on how to detect if building with musl...

baruchsiach commented 6 years ago

There is no libc specific macro that musl libc defines. This is an explicit decision of the musl maintainer.

Note that musl does provide __c_ispeed and __c_ospeed. Not sure if that gives much in the way of help.

npat-efault commented 6 years ago

Yes I saw this stupidity googling for it.

Then the solution would be to build with dafault custom baudrate support only for libc's we recognize, or check for feature macros like HAVE_STRUCT_TERMIOS_C_ISPEED which are provided by the other libs.

Yes __c_ispeed and __c_ospeed would help, IF there was a way to check for musl...

npat-efault commented 6 years ago

@baruchsiach Can you test the latest tip and report back? Commit 1acf1ddabaf3576b4023c4f6f09c5a3e4b086fb8 should fix the issue.

baruchsiach commented 6 years ago

I can confirm that this commit fixes build with musl.

I updated the Buildroot fix to use this patch backported to 3.1: http://patchwork.ozlabs.org/patch/897731/.

nolange commented 4 years ago

I seen this too late, built my own hackaround for building with musl (based on v1.3), which does not disable custom baud. (I would not know how to test the functionality however)

Musl cant be detected with macros, so a clean version would test if code compiles with _c[io]speed, then define a macro for these workarounds

diff -burN picocom-3.1.org/termbits2.h picocom-3.1/termbits2.h
--- picocom-3.1.org/termbits2.h 2018-02-01 10:20:02.000000000 +0100
+++ picocom-3.1/termbits2.h 2020-01-14 18:41:51.937596049 +0100
@@ -34,6 +34,26 @@
 /* We need tcflag_t, cc_t, speed_t, CBAUDEX, etc */
 #include <termios.h>

+#ifndef __MAX_BAUD
+#define __MAX_BAUD B4000000
+#endif
+
+#ifndef TCGETS2
+#define TCGETS2   _IOR('T', 0x2A, struct termios2)
+#define TCSETS2   _IOW('T', 0x2B, struct termios2)
+#define TCSETSW2  _IOW('T', 0x2C, struct termios2)
+#define TCSETSF2  _IOW('T', 0x2D, struct termios2)
+#endif
+
+#if defined(__linux__) && (defined(__GLIBC__) || defined(__UCLIBC__))
+#define cfgetospeed_custom(tiop) ((tiop)->c_ospeed)
+#define cfgetispeed_custom(tiop) ((tiop)->c_ispeed)
+#else
+#define cfgetospeed_custom(tiop) ((tiop)->__c_ospeed)
+#define cfgetispeed_custom(tiop) ((tiop)->__c_ispeed)
+#endif
+
+
 /* These definitions must correspond to the kernel structures as
    defined in:

diff -burN picocom-3.1.org/termios2.c picocom-3.1/termios2.c
--- picocom-3.1.org/termios2.c  2018-02-01 10:20:02.000000000 +0100
+++ picocom-3.1/termios2.c  2020-01-14 18:40:29.636966602 +0100
@@ -79,8 +79,8 @@
     t2.c_cflag = tios->c_cflag;
     t2.c_lflag = tios->c_lflag;
     t2.c_line = tios->c_line;
-    t2.c_ispeed = tios->c_ispeed;
-    t2.c_ospeed = tios->c_ospeed;
+    t2.c_ispeed = cfgetispeed_custom(tios);
+    t2.c_ospeed = cfgetospeed_custom(tios);
     memcpy(&t2.c_cc[0], &tios->c_cc[0], K_NCCS * sizeof (cc_t));

     return ioctl(fd, cmd, &t2);
@@ -101,8 +101,8 @@
     tios->c_cflag = t2.c_cflag;
     tios->c_lflag = t2.c_lflag;
     tios->c_line = t2.c_line;
-    tios->c_ispeed = t2.c_ispeed;
-    tios->c_ospeed = t2.c_ospeed;
+    cfgetispeed_custom(tios) = t2.c_ispeed;
+    cfgetospeed_custom(tios) = t2.c_ospeed;
     memcpy(&tios->c_cc[0], &t2.c_cc[0], K_NCCS * sizeof (cc_t));

     for (i = K_NCCS; i < NCCS; i++)
@@ -131,7 +131,7 @@
         errno = EINVAL;
         return -1;
     }
-    tios->c_ispeed = speed;
+    cfgetispeed_custom(tios) = speed;
     tios->c_cflag &= ~((CBAUD | CBAUDEX) << IBSHIFT);
     tios->c_cflag |= (speed << IBSHIFT);

@@ -156,7 +156,7 @@
     }
     tios->c_cflag &= ~(CBAUD | CBAUDEX);
     tios->c_cflag |= BOTHER;
-    tios->c_ospeed = speed;
+    cfgetospeed_custom(tios) = speed;

     return 0;
 }
@@ -177,7 +177,7 @@
     } else {
         tios->c_cflag &= ~((CBAUD | CBAUDEX) << IBSHIFT);
         tios->c_cflag |= (BOTHER << IBSHIFT);
-        tios->c_ispeed = speed;
+        cfgetispeed_custom(tios) = speed;
     }

     return 0;
diff -burN picocom-3.1.org/termios2.h picocom-3.1/termios2.h
--- picocom-3.1.org/termios2.h  2018-02-01 10:20:02.000000000 +0100
+++ picocom-3.1/termios2.h  2020-01-14 18:38:23.423996565 +0100
@@ -37,8 +37,13 @@
 /* And define these new ones */
 #define cfsetospeed_custom cf2setospeed_custom
 #define cfsetispeed_custom cf2setispeed_custom
+#if defined(__linux__) && (defined(__GLIBC__) || defined(__UCLIBC__))
 #define cfgetospeed_custom(tiop) ((tiop)->c_ospeed)
 #define cfgetispeed_custom(tiop) ((tiop)->c_ispeed)
+#else
+#define cfgetospeed_custom(tiop) ((tiop)->__c_ospeed)
+#define cfgetispeed_custom(tiop) ((tiop)->__c_ispeed)
+#endif

 /* Replacements for the standard tcsetattr(3), tcgetattr(3)
  * functions. Same user interface, but these use the new termios2