[PATCH 7/9] Patching STTY command for use in RTEMS
Kirspel, Kevin
Kevin-Kirspel at idexx.com
Thu Feb 9 21:51:02 UTC 2017
Before or after the "Build the libbsd without optimization." step, I would add a statement about fixing up any compilation errors. Maybe most commands will compile after making the changes listed previously but I had to fixup errors related to termios differences between RTEMS and FREEBSD. Besides that, the instructions were fine and I got the suspected output.
I didn't realize the files were being changed to executables. I edit the files from a windows application running over a Samba share. I'll check and see if samba is making the change on save.
I will fix the tabs. Sometimes I forget to change my editor settings for the different files I am working on.
There is an issue with the getopt_r() function. Even if you initialize the optind value to 1 the getopt_r() function returns '?'. So you get past the strspn() check but fail the getopt_r() call. If you initialize the optind value to 0, you get the right return value but the optind is wrong for the strspn() call. If you change the original code to the following, then it will also work:
if (strspn(argv[optind == 0 ? 1 : optind], "-aefg") == strlen(argv[optind == 0 ? 1 : optind])
Kevin Kirspel
Electrical Engineer - Sr. Staff
Idexx Roswell
235 Hembree Park Drive
Roswell GA 30076
Tel: (770)-510-4444 ext. 81642
Direct: (770)-688-1642
Fax: (770)-510-4445
-----Original Message-----
From: Christian Mauderer [mailto:christian.mauderer at embedded-brains.de]
Sent: Thursday, February 09, 2017 3:36 PM
To: Kirspel, Kevin <Kevin-Kirspel at idexx.com>
Cc: RTEMS Devel <devel at rtems.org>
Subject: Re: [PATCH 7/9] Patching STTY command for use in RTEMS
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 https://urldefense.proofpoint.com/v2/url?u=http-3A__pubs.opengroup.org_onlinepubs_9699919799_functions_getopt.html&d=DwIFaQ&c=2do6VJGs3LvEOe4OFFM1bA&r=HDiJ93ANMEQ32G5JGdpyUxbdebuwKHBbeiHMr3RbR74&m=GP0OdJY1ZudYsz_Sa37k-2CMMp1GBHHp5R-7pDdgqzI&s=7caspKinrmx0H4JqdzPYRp3--HEIMqctvGpTQ0XmDGo&e= ) 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.
More information about the devel
mailing list