Re: [PATCH 7/9] Patching STTY command for use in RTEMS
Am 10.02.2017 um 15:06 schrieb Kirspel, Kevin: > 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. I think it might be worth to raise an issue at the newlib but it is quite possible that patching this would break a number of other existing applications (not only RTEMS but others that use newlib) so I'm not sure whether the newlib developers are really found of this idea. Kind regards Christian Mauderer > > Kevin Kirspel > Electrical Engineer - Sr. Staff > Idexx Roswell > 235 Hembree Park Drive > Roswell GA 30076 > Tel: (770)-510- 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 > Cc: RTEMS Devel > 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- 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 >> Cc: RTEMS Devel >> 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" >>> An: "RTEMS Devel" >>> 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 >>> >>> +#ifdef __rtems__ >>> +#include "rtems-bsd-stty-namespace.h" >>
RE: [PATCH 7/9] Patching STTY command for use in RTEMS
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- 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 Cc: RTEMS Devel 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- 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 > Cc: RTEMS Devel > 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" >> An: "RTEMS Devel" >> 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 >> >> +#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.c8.3 (Berkeley) >> 4/2/94"; >> #include >> __FBSDID("$FreeBSD$"); >> >> +#ifdef __rtems__ >> +#define __need_getopt_newlib >> +#include >> +#include #include >> + #endif /* __rtems__ */ >> #include >> >> #include >> @@ -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 m
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- 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 > Cc: RTEMS Devel > 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" >> An: "RTEMS Devel" >> 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 >> >> +#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.c8.3 (Berkeley) >> 4/2/94"; >> #include >> __FBSDID("$FreeBSD$"); >> >> +#ifdef __rtems__ >> +#define __need_getopt_newlib >> +#include >> +#include #include >> + #endif /* __rtems__ */ >> #include >> >> #include >> @@ -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); >> + rte
RE: [PATCH 7/9] Patching STTY command for use in RTEMS
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- 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 Cc: RTEMS Devel 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" > An: "RTEMS Devel" > 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 > > +#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 > __FBSDID("$FreeBSD$"); > > +#ifdef __rtems__ > +#define __need_getopt_newlib > +#include > +#include #include > + #endif /* __rtems__ */ > #include > > #include > @@ -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 = "
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" > An: "RTEMS Devel" > 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 > > +#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 > __FBSDID("$FreeBSD$"); > > +#ifdef __rtems__ > +#define __need_getopt_newlib > +#include > +#include > +#include > +#endif /* __rtems__ */ > #include > > #include > @@ -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; > +