On Wed, 2018-03-07 at 13:56 +0100, Christophe de Dinechin wrote:
> > On 7 Mar 2018, at 07:40, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> >
> > >
> > > Introduced in 548577dc8adae1a558
> > >
> > > Signed-off-by: Uri Lublin <uril@xxxxxxxxxx>
> > > ---
> > > src/spice-streaming-agent.cpp | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> > > index b39782c..e25d47a 100644
> > > --- a/src/spice-streaming-agent.cpp
> > > +++ b/src/spice-streaming-agent.cpp
> > > @@ -145,6 +145,11 @@ static void handle_stream_error(size_t len)
> > > std::to_string(sizeof(StreamMsgNotifyError))
> > > + ")");
> > > }
> > >
> > > + // This struct inherits StreamMsgNotifyError. Its memory layout is:
> > > + // offset 0: StreamMsgNotifyError.error_code (a uint32_t)
> > > + // offset 4: StreamMsgNotifyError.msg and also msg.msg (a
> > > uint8_t[1024]).
> > > + // Both StreamMsgNotifyError.msg and msg.msg point to the same
> > > + // memory location (practically local msg overrides inherited msg).
> > > struct : StreamMsgNotifyError {
> > > uint8_t msg[1024];
> > > } msg;
> >
> > When I sent my suggestion I didn't wanted to clash with inherited field,
> > just didn't invented a new name.
> > Maybe would be easier and more clear to rename to msg_buffer or similar
> > to avoid the confusion?
>
> Would fix the issue I pointed out. But as I mentioned there, gcc emkits a warning that we access the array out of bounds (msg has zero length). You can silence that using a temporary char * pointer.
Are you talking about the same thing as Frediano? IIUC:
- Frediano says rename msg.msg to msg.msg_buffer and use that
- You talk about out of bounds warning for StreamMsgNotifyError::msg
and casting it to a pointer to avoid that (so that we wouldn't need to
inherit the struct at all)
To sum it up, what changes are requested?
- Fix msg.msg in a TBD way.
- If the message is too long, read it out instead of throwing an
exception to prevent a DoS. (We can still get DoSed though if the
server sends a neverending message or whatever?)
- If the inherited struct stays, name it? (I have to say I liked the
unnamed struct in local scope as it limited it's usage exactly to the
one 'msg' variable...)
Cheers,
Lukas
> >
> > Frediano
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel