I can include the libbsd.txt updates in my updated patches. If you want getopt_r() to match FREEBSD, it needs to be updated so that it works correctly when optind is initialized to 1 or 0.
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.maude...@embedded-brains.de] Sent: Friday, February 10, 2017 6:04 AM To: Kirspel, Kevin <kevin-kirs...@idexx.com> Cc: RTEMS Devel <devel@rtems.org> Subject: Re: [PATCH 7/9] Patching STTY command for use in RTEMS Hello Kevin, Am 09.02.2017 um 22:51 schrieb Kirspel, Kevin: > 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. OK. So in general it worked. That is good to know. Do you want to add this information to the documentation or should I create a patch? > > 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 assumed such a problem. Also it's just a detail. > > I will fix the tabs. Sometimes I forget to change my editor settings for the > different files I am working on. Thanks. > > 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]) OK. So if I understand you correctly, only the first value is really wrong but the optind will have the correct value for the further processing farther below. That was my main concern. > > 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.maude...@embedded-brains.de] > Sent: Thursday, February 09, 2017 3:36 PM > To: Kirspel, Kevin <kevin-kirs...@idexx.com> > Cc: RTEMS Devel <devel@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-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 > 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.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. > -- -------------------------------------------- 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
