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-kirs...@idexx.com>
> An: "RTEMS Devel" <devel@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.maude...@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.
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
