Re: [PATCH V4] PR gdb/33747: gdb/ser-unix: modernize and fix custom baud rate support on musl

["Maciej W. Rozycki" <macro@orcam.me.uk>]

On Mon, 23 Mar 2026, sunilkumar.dora@windriver.com wrote:

> Fix this at the macro level by requiring HAVE_STRUCT_TERMIOS_C_OSPEED
> alongside BOTHER in the HAVE_CUSTOM_BAUDRATE_SUPPORT guard.  This
> prevents the Linux-specific path from being compiled on systems where
> the required fields are unavailable, leaving set_custom_baudrate_linux()
> unchanged.

 The GNU coding style avoids using parentheses in documentation references 
to functions except for actual function calls with no arguments.

> This interface is supported by glibc 2.42 and later and is expected to
> be standardized by a future POSIX revision.  It also covers systems
> such as GNU Hurd that do not provide BOTHER.  Return values from
> cfsetispeed() and cfsetospeed() are checked, and errors are reported
> via perror_with_name().

 Same here.

> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index cf8078e1d89..e558cf44703 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -725,6 +725,28 @@ AC_CONFIG_FILES([jit-reader.h:jit-reader.in])
>  
>  AC_SEARCH_LIBS(dlopen, dl)
>  
> +# Check whether cfsetispeed/cfsetospeed accept arbitrary baud rates.
> +AC_CACHE_CHECK([whether cfsetispeed/cfsetospeed accept arbitrary baud rates],
> +  [gdb_cv_cfsetspeed_arbitrary], [
> +  AC_COMPILE_IFELSE(
> +    [AC_LANG_PROGRAM([[#include <termios.h>]],
> +      [[
> +       #if B9600 != 9600
> +       #error B-constants are not numeric symbols
> +       #endif
> +      ]])],
> +    [gdb_cv_cfsetspeed_arbitrary=yes],
> +    [gdb_cv_cfsetspeed_arbitrary=no])
> +])
> +
> +if test "$gdb_cv_cfsetspeed_arbitrary" = yes; then
> +  AC_DEFINE([HAVE_CFSETSPEED_ARBITRARY], [1],
> +	    [Define if cfsetispeed/cfsetospeed accept arbitrary baud rates])

 Missing full stop in the description.

> diff --git a/gdb/ser-unix.c b/gdb/ser-unix.c
> index c295a9c5ba1..0a6cdf0f630 100644
> --- a/gdb/ser-unix.c
> +++ b/gdb/ser-unix.c
> @@ -55,7 +55,10 @@
>  
>  #include "gdbsupport/scoped_ignore_sigttou.h"
>  
> -#if defined(HAVE_SYS_IOCTL_H) && (defined(BOTHER) || defined(IOSSIOSPEED))
> +#if defined(HAVE_CFSETSPEED_ARBITRARY) || \
> +    (defined(HAVE_SYS_IOCTL_H) && \
> +     ((defined(BOTHER) && defined(HAVE_STRUCT_TERMIOS_C_OSPEED)) || \
> +      defined(IOSSIOSPEED)))

 The GNU coding style is to have lines split before an operator, and extra
parentheses added where necessary for the indentation to show nesting, 
i.e.:

#if (defined(HAVE_CFSETSPEED_ARBITRARY) \
     || (defined(HAVE_SYS_IOCTL_H) \
	 && ((defined(BOTHER) && defined(HAVE_STRUCT_TERMIOS_C_OSPEED)) \
	     || defined(IOSSIOSPEED))))

> @@ -508,6 +511,33 @@ set_baudcode_baudrate (struct serial *scb, int baud_code)
>      perror_with_name (_("could not set tty state"));
>  }
>  
> +#if defined(HAVE_CFSETSPEED_ARBITRARY)

 Please make it consistent with the existing convention, so:

#if HAVE_CUSTOM_BAUDRATE_SUPPORT && defined(HAVE_CFSETSPEED_ARBITRARY)

...

> +
> +/* Set a custom baud rate using the POSIX cfsetispeed/cfsetospeed
> +   interface.  Supported in glibc 2.42+ and expected to be
> +   standardized in a future POSIX revision.  It is platform-agnostic
> +   and also covers systems like GNU Hurd that do not provide BOTHER.  */
> +
> +static void
> +set_custom_baudrate_posix (int fd, int rate)
> +{
> +  struct termios tio;
> +
> +  if (tcgetattr (fd, &tio) < 0)
> +    perror_with_name (_("Can not get current baud rate"));
> +
> +  if (cfsetispeed (&tio, rate) < 0)
> +    perror_with_name (_("Can not set custom input baud rate"));
> +
> +  if (cfsetospeed (&tio, rate) < 0)
> +    perror_with_name (_("Can not set custom output baud rate"));
> +
> +  if (tcsetattr (fd, TCSANOW, &tio) < 0)
> +    perror_with_name (_("Can not set custom baud rate"));
> +}
> +
> +#endif /* HAVE_CFSETSPEED_ARBITRARY */
> +
>  #if HAVE_CUSTOM_BAUDRATE_SUPPORT && defined(BOTHER)

... and then:

#elif HAVE_CUSTOM_BAUDRATE_SUPPORT && defined(BOTHER)

(and then update the comment for the trailing #endif accordingly).

 I think it would best be done as two separate changes in a small patch 
series, first to fix the compilation error with musl, and second to add 
the POSIX interface, so that each is a self-contained feature.

  Maciej