[PATCH 7/9] Patching STTY command for use in RTEMS

[Christian Mauderer]

Hello Kevin,

thanks for your work.

Out of curiosity: It seems that you are one of the first users of the new porting process for user space applications (beneath Sebastian and me). Did you encounter any problems that might be worth adding to the guide in libbsd.txt?

Beneath that please see my remarks below.

Kind regards

Christian

----- Ursprüngliche Mail -----
> Von: "Kevin Kirspel" <kevin-kirspel at idexx.com>
> An: "RTEMS Devel" <devel at rtems.org>
> Gesendet: Donnerstag, 9. Februar 2017 04:21:38
> Betreff: [PATCH 7/9] Patching STTY command for use in RTEMS

[...]
> diff --git a/freebsd/bin/stty/stty.c b/freebsd/bin/stty/stty.c
> old mode 100644
> new mode 100755

You made the files executable. It would be better, if you could avoid that. Please note that this is also true for a lot of other sources in your patches.

> index 54c63f6..3999a7f
> --- a/freebsd/bin/stty/stty.c
> +++ b/freebsd/bin/stty/stty.c
> @@ -1,5 +1,8 @@
> #include <machine/rtems-bsd-user-space.h>
> 
> +#ifdef __rtems__
> +#include "rtems-bsd-stty-namespace.h"
> +#endif /* __rtems__ */
> /*-
>  * Copyright (c) 1989, 1991, 1993, 1994
>  *	The Regents of the University of California.  All rights reserved.
> @@ -43,6 +46,12 @@ static char sccsid[] = "@(#)stty.c	8.3 (Berkeley) 4/2/94";
> #include <sys/cdefs.h>
> __FBSDID("$FreeBSD$");
> 
> +#ifdef __rtems__
> +#define __need_getopt_newlib
> +#include <getopt.h>
> +#include <machine/rtems-bsd-program.h>
> +#include <machine/rtems-bsd-commands.h>
> +#endif /* __rtems__ */
> #include <sys/types.h>
> 
> #include <ctype.h>
> @@ -57,20 +66,56 @@ __FBSDID("$FreeBSD$");
> 
> #include "stty.h"
> #include "extern.h"
> +#ifdef __rtems__
> +#include "rtems-bsd-stty-stty-data.h"
> +#endif /* __rtems__ */
> +
> +#ifdef __rtems__
> +static int main(int argc, char *argv[]);
> +
> +RTEMS_LINKER_RWSET(bsd_prog_stty, char);
> 
> int
> +rtems_bsd_command_stty(int argc, char *argv[])
> +{
> +  int exit_code;
> +  void *data_begin;
> +  size_t data_size;
> +
> +  data_begin = RTEMS_LINKER_SET_BEGIN(bsd_prog_stty);
> +  data_size = RTEMS_LINKER_SET_SIZE(bsd_prog_stty);
> +
> +  rtems_bsd_program_lock();
> +  exit_code = rtems_bsd_program_call_main_with_data_restore("stty",
> +      main, argc, argv, data_begin, data_size);
> +  rtems_bsd_program_unlock();
> +
> +  return exit_code;
> +}

In most (all?) places the libbsd uses BSD coding style. This is especially true for code that is inserted into existing BSD code. In this case that means one tab instead of two spaces. I also noted that on some of the other patches in the patch set.

> +#endif /* __rtems__ */
> +int
> main(int argc, char *argv[])
> {
> 	struct info i;
> 	enum FMT fmt;
> 	int ch;
> 	const char *file, *errstr = NULL;
> +#ifdef __rtems__
> +	struct getopt_data getopt_data;
> +	memset(&getopt_data, 0, sizeof(getopt_data));
> +#define optind getopt_data.optind
> +#define optarg getopt_data.optarg
> +#define opterr getopt_data.opterr
> +#define optopt getopt_data.optopt
> +#define getopt(argc, argv, opt) getopt_r(argc, argv, "+" opt, &getopt_data)
> +#endif /* __rtems__ */
> 
> 	fmt = NOTSET;
> 	i.fd = STDIN_FILENO;
> 	file = "stdin";
> 
> 	opterr = 0;
> +#ifndef __rtems__
> 	while (optind < argc &&
> 	    strspn(argv[optind], "-aefg") == strlen(argv[optind]) &&
> 	    (ch = getopt(argc, argv, "aef:g")) != -1)
> @@ -93,6 +138,34 @@ main(int argc, char *argv[])
> 		default:
> 			goto args;
> 		}
> +#else /* __rtems__ */
> +	while (optind < argc && (ch = getopt(argc, argv, "aef:g")) != -1) {
> +		int optidx = optind - ((optarg == 0) ? 1 : 2);
> +		if(strspn(argv[optidx], "-aefg") == strlen(argv[optidx])) {

I'm really not sure about that one. Eventually you could help me understand it. Why was the change necessary?

I assume that it has to do something with the problem that optind should normally be initialized with 1 (according to http://pubs.opengroup.org/onlinepubs/9699919799/functions/getopt.html) but in our case it is 0 due to the memset. Is that correct? Would the value for the further loop cycles be correct or is there a general problem that the newlib optind is one off?

A few lines further down the original code has the following two lines:

        args:	argc -= optind;
        	argv += optind;

If you had a problem with optind in the query: Has it the correct value here?

> +			switch(ch) {
> +			case 'a':   /* undocumented: POSIX compatibility */
> +				fmt = POSIX;
> +				break;
> +			case 'e':
> +				fmt = BSD;
> +				break;
> +			case 'f':
> +				if ((i.fd = open(optarg, O_RDONLY | O_NONBLOCK)) < 0)
> +					err(1, "%s", optarg);
> +				file = optarg;
> +				break;
> +			case 'g':
> +				fmt = GFLAG;
> +				break;
> +			case '?':
> +			default:
> +				goto args;
> +			}
> +		} else {
> +			break;
> +		}
> +	}
> +#endif /* __rtems__ */
> 
> args:	argc -= optind;
> 	argv += optind;
> @@ -136,8 +209,13 @@ args:	argc -= optind;
> 			speed = strtonum(*argv, 0, UINT_MAX, &errstr);
> 			if (errstr)
> 				err(1, "speed");
> +#ifndef __rtems__
> 			cfsetospeed(&i.t, speed);
> 			cfsetispeed(&i.t, speed);
> +#else /* __rtems__ */
> +			cfsetospeed(&i.t, rtems_bsd_bsd_speed_to_rtems_speed(speed));
> +			cfsetispeed(&i.t, rtems_bsd_bsd_speed_to_rtems_speed(speed));
> +#endif /* __rtems__ */
> 			i.set = 1;
> 			continue;
> 		}
[...]

-- 
--------------------------------------------
embedded brains GmbH
Christian Mauderer
Dornierstr. 4
D-82178 Puchheim
Germany
email: christian.mauderer at embedded-brains.de
Phone: +49-89-18 94 741 - 18
Fax:   +49-89-18 94 741 - 08
PGP: Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.