[Spice-devel] [PATCH spice-streaming-agent v2 4/4] better error message when opening streaming device

[Lukáš Hrázký]

On Thu, 2018-01-25 at 07:29 -0500, Frediano Ziglio wrote:
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> > ---
> >  src/spice-streaming-agent.cpp | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-
> > agent.cpp
> > index 87e8fa3..0200e4a 100644
> > --- a/src/spice-streaming-agent.cpp
> > +++ b/src/spice-streaming-agent.cpp
> > @@ -356,7 +356,8 @@ do_capture(const string &streamport, FILE
> > *f_log)
> >      streamfd = open(streamport.c_str(), O_RDWR);
> >      if (streamfd < 0)
> >          // TODO was syslog(LOG_ERR, "Failed to open %s: %s\n",
> > streamport,
> >          strerror(errno));
> > -        throw std::runtime_error("failed to open streaming
> > device");
> > +        throw std::runtime_error("failed to open the streaming
> > device (" +
> > +            streamport + "): " + string(strerror(errno)));
> > 
> 
> The last string constructor explicit call is not necessary.

That's right, I'll remove it.

> Are we use that when we'll read errno will still contain the error
> from open?
> Maybe it works but potentially can be overwritten at that time, code
> needs
> to call different constructors/add operators before getting errno and
> call
> strerror.

Well, no direct constructor call is actually made in this case before
reading the errno, only the operator+ is called. While it may overwrite
errno at least in case it fails to allocate memory, it will also in
such case throw an(other) exception, so the errno read at the end of
the line will not actually happen.

> I think also this commit fix the above TODO which could then be
> removed.

I wasn't sure at all what the TODO was actually about :) Will remove
it.

> We usually align after the "(" but I'm fine with this alignment.
>  
> >      unsigned int frame_count = 0;
> >      while (! quit) {

That's my habit, aligning to an arbitrary-positioned "(" is not my
favorite :) I can 'fix' it though :)

Lukas