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

Reply via email to