>
> On Wed, 2018-02-21 at 05:45 -0500, Frediano Ziglio wrote:
> > >
> > > On Tue, 2018-02-20 at 20:48 +0000, Frediano Ziglio wrote:
> > > > Prepare to add support for other messages.
> > > >
> > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > > ---
> > > > src/spice-streaming-agent.cpp | 26 ++++++++++++++++++--------
> > > > 1 file changed, 18 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/src/spice-streaming-agent.cpp
> > > > b/src/spice-streaming-agent.cpp
> > > > index 5613934..8d91f2d 100644
> > > > --- a/src/spice-streaming-agent.cpp
> > > > +++ b/src/spice-streaming-agent.cpp
> > > > @@ -77,10 +77,11 @@ static int have_something_to_read(int timeout)
> > > > return 0;
> > > > }
> > > >
> > > > +static void handle_stream_start_stop(uint32_t len);
> > > > +
> > >
> > > Why not put the method here instead of a forward declaration (and
> > > adding more in the future)?
> > >
> >
> > Easier to read if you read the file from top to bottom.
>
> I don't think so, because since functions always need to be declared
> before use, the natural structure of a source file always is from
> bottom to top. People are used to it. This seems to me like an
> exception that actually breaks the flow. It appears there should be
> another reason to reverse the order and do a forward declaration, while
> in fact there isn't any.
>
Not strong about, I'll revert.
> It is bound to be refactored anyway, but still...
>
> > > > static void read_command_from_device(void)
> > > > {
> > > > StreamDevHeader hdr;
> > > > - uint8_t msg[64];
> > > > int n;
> > > >
> > > > std::lock_guard<std::mutex> stream_guard(stream_mtx);
> > > > @@ -93,17 +94,26 @@ static void read_command_from_device(void)
> > > > throw std::runtime_error("BAD VERSION " +
> > > > std::to_string(hdr.protocol_version) +
> > > > " (expected is " +
> > > > std::to_string(STREAM_DEVICE_PROTOCOL)
> > > > +
> > > > ")");
> > > > }
> > > > - if (hdr.type != STREAM_TYPE_START_STOP) {
> > > > - throw std::runtime_error("UNKNOWN msg of type " +
> > > > std::to_string(hdr.type));
> > > > +
> > > > + switch (hdr.type) {
> > > > + case STREAM_TYPE_START_STOP:
> > > > + return handle_stream_start_stop(hdr.size);
> > > > }
> > > > - if (hdr.size >= sizeof(msg)) {
> > > > - throw std::runtime_error("msg size (" +
> > > > std::to_string(hdr.size) +
> > > > ") is too long "
> > > > + throw std::runtime_error("UNKNOWN msg of type " +
> > > > std::to_string(hdr.type));
> > > > +}
> > > > +
> > > > +static void handle_stream_start_stop(uint32_t len)
> > > > +{
> > > > + uint8_t msg[256];
> > > > +
> > > > + if (len >= sizeof(msg)) {
> > > > + throw std::runtime_error("msg size (" + std::to_string(len) +
> > > > ")
> > > > is too long "
> > > > "(longer than " +
> > > > std::to_string(sizeof(msg)) + ")");
> > > > }
> > > > - n = read(streamfd, &msg, hdr.size);
> > > > - if (n != hdr.size) {
> > > > + int n = read(streamfd, &msg, len);
> > > > + if (n != len) {
> > > > throw std::runtime_error("read command from device FAILED --
> > > > read
> > > > " + std::to_string(n) +
> > > > - " expected " +
> > > > std::to_string(hdr.size));
> > > > + " expected " + std::to_string(len));
> > > > }
> > > > streaming_requested = (msg[0] != 0); /* num_codecs */
> > > > syslog(LOG_INFO, "GOT START_STOP message -- request to %s
> > > > streaming\n",
> > >
> > > Apart from the forward declaration,
> > >
> > > Acked-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> > >
> >
> > Frediano
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel