[Spice-devel] [PATCH spice-streaming-agent v2 4/4] log_binary is really a boolean

[Frediano Ziglio]

> > On 15 Feb 2018, at 11:00, Frediano Ziglio <fziglio at redhat.com> wrote:
> > 
> > From: Christophe de Dinechin <dinechin at redhat.com>
> > 
> > Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> > ---
> > Change since v1:
> > - do not clash with possible short 'b' option.
> > ---
> > src/spice-streaming-agent.cpp | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > index 4ec5e42..aa73826 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -55,9 +55,9 @@ struct SpiceStreamDataMessage
> > 
> > static bool streaming_requested = false;
> > static bool quit_requested = false;
> > +static bool log_binary = false;
> > static std::set<SpiceVideoCodecType> client_codecs;
> > static int streamfd = -1;
> > -static int log_binary = 0;
> > static std::mutex stream_mtx;
> > 
> > static int have_something_to_read(int timeout)
> > @@ -405,11 +405,14 @@ done:
> > int main(int argc, char* argv[])
> > {
> >     std::string streamport = "/dev/virtio-ports/com.redhat.stream.0";
> > -    char opt;
> > +    int opt;
> >     const char *log_filename = NULL;
> >     int logmask = LOG_UPTO(LOG_WARNING);
> > -    struct option long_options[] = {
> > -        { "log-binary", no_argument, &log_binary, 1},
> > +    enum {
> > +        OPT_LOG_BINARY = UCHAR_MAX+1
> > +    };
> > +    static const struct option long_options[] = {
> > +        { "log-binary", no_argument, NULL, OPT_LOG_BINARY},
> 
> Wouldn’t it be simpler to add the -b option for binary log, since we don’t
> use it?
> 
> That was my intent, and then it slipped my mind…
> 

This patch is about style and type, adding an option is a public
interface change.
I don't think that this option that requires a short option.

> 
> >         { "help", no_argument, NULL, 'h'},
> >         { 0, 0, 0, 0}
> >     };
> > @@ -437,6 +440,9 @@ int main(int argc, char* argv[])
> >             agent.AddOption(optarg, p);
> >             break;
> >         }
> > +        case OPT_LOG_BINARY:
> > +            log_binary = true;
> > +            break;
> >         case 'l':
> >             log_filename = optarg;
> >             break;

Frediano