Opened 3 years ago

Last modified 8 months ago

#1256 new defect

Minor error in getopt() processing

Reported by: shane Owned by:
Priority: very low Milestone: DNS Outstanding Tasks
Component: b10-auth Keywords:
Cc: CVSS Scoring:
Sensitive: no Defect Severity: Low
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

According to the getopt() man page:

       If getopt() finds an option character in argv that was not included  in
       optstring,  or  if it detects a missing option argument, it returns '?'
       and sets the external variable optopt to the actual  option  character.
       If  the  first  character  (following any optional '+' or '-' described
       above) of optstring is a colon (':'), then getopt() returns ':' instead
       of  '?'  to  indicate  a  missing  option  argument.   If  an error was
       detected, and the first character of optstring is not a colon, and  the
       external  variable  opterr  is nonzero (which is the default), getopt()
       prints an error message.

We use the leading colon sometimes, I think in an effort to avoid the pre-defined error messages, which are ugly and not helpful. However, we then check for '?' in our while() loop, which is an error.

It doesn't actually show up, since we then follow with the default: statement, however a restructuring of the code could break this. It's a one-line fix.

Change History (8)

comment:1 Changed 3 years ago by shane

  • Milestone changed from New Tasks to Next-Sprint-Proposed

comment:2 Changed 3 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Year 3 Task Backlog

comment:3 follow-up: Changed 3 years ago by jreed

So do you want to check for : (in the case) instead or get rid of the colon (in the getopt)?

Do we want it to complain about the unknown option or not (in addition to giving the usage statement)?

Note that the src/bin/dhcp6/main.cc uses the colon and checks for the colon, but doesn't yet have any switches that have an option argument.

The colon magic, in my opinion, is not good as it also loses the explanation specific to the problem, such as b10-auth: option requires an argument -- u

comment:4 in reply to: ↑ 3 ; follow-up: Changed 3 years ago by shane

Replying to jreed:

So do you want to check for : (in the case) instead or get rid of the colon (in the getopt)?

I'd like to keep the ':' check which we have in the getopt() function call argument, and make the switch statement following it also check for ':' - instead of '?'.

Do we want it to complain about the unknown option or not (in addition to giving the usage statement)?

Yes, we do.

Note that the src/bin/dhcp6/main.cc uses the colon and checks for the colon, but doesn't yet have any switches that have an option argument.

Okay, I think this is probably okay as it won't hurt anything, and may help prevent forgetting it!

The colon magic, in my opinion, is not good as it also loses the explanation specific to the problem, such as b10-auth: option requires an argument -- u

Yeah, the problem is that the default message is ugly and non-helpful. Straight from 1971, I think. :)

Ideally we would use a more modern replacement for getopt(), I think that this ticket may need to investigate that possibility. I see that there is boost::program_options, which may be what we want to use.

http://www.boost.org/doc/libs/1_37_0/doc/html/program_options.html

comment:5 in reply to: ↑ 4 Changed 3 years ago by jinmei

Replying to shane:

Ideally we would use a more modern replacement for getopt(), I think that this ticket may need to investigate that possibility. I see that there is boost::program_options, which may be what we want to use.

http://www.boost.org/doc/libs/1_37_0/doc/html/program_options.html

boost::program_options requires a compiled library (unless it has
changed in a very recent version). That's why we are not using it.

comment:6 Changed 3 years ago by shane

See also ticket #528.

comment:7 Changed 3 years ago by shane

There are a few possible alternatives. Perhaps the easiest might be UltraGetopt, which appears to be a work-alike with an MIT license that is loved and maintained:

http://kevinlocke.name/programs/ultragetopt.php

Apparently popt is popular (the documentation is buried in a PDF in the release):

http://rpm5.org/files/popt/

And there is also a Google library:

http://code.google.com/p/google-gflags/

comment:8 Changed 8 months ago by stephen

  • Milestone set to DNS Outstanding Tasks
Note: See TracTickets for help on using tickets.