> > Hi, > > On Thu, Jun 08, 2017 at 10:39:53AM -0400, Frediano Ziglio wrote: > > > > > > On Thu, Jun 08, 2017 at 03:55:11PM +0200, Victor Toso wrote: > > > > > > > > > > Not really worth all that discussion, your patch is ok :) > > > > > > > > Ah, I liked the discusison mostly because this would be a spice <-> > > > > spice-gtk communication bug and I'm not sure either if we should go for > > > > CRITICAL or WARNING messages in such cases. > > > > > > > > I mean, in the past, for spice server, there was discussion about when > > > > to assert(). The result was that only when it would be a bug inside the > > > > component (never on client's input/configuration for instance) > > > > > > > > Anyway, thanks for the discussion :) > > > > > > To tie this in with the logging discussion, you can read the latest > > > comment from ebassi in this blog post about g_log_structured ;) > > > > > > https://blog.gtk.org/2017/05/04/logging-and-more/ > > > > > > Christophe > > > > > > > Wrong thread? Looks like the structured logging would be more > > appropriate in Chistophe D thread. > > > > About CRITICAL or WARNING personally I think if you are detecting > > that server for some reason is sending garbage telling the used on > > the UI and closing the connection/program is a good solution for > > an user application. > > Not really garbage but likely a bug. In this specific case we should > drop the stream and server should handle it gracefully too, no need to > assert or quit the application because of that. > > Both warning and critical are fine, I guess. Documentation says: > > "g_return_val_if_reached()) should only be used for programming > errors, a typical use case is checking for invalid parameters at the > beginning of a public function. They should not be used if you just > mean "if (error) return"" > > I would say that this can be checked as programming error and not just > any-type-of-error, for that reason I'm still keeping the > g_return_if_fail() > > toso > > > For the server is a completely different case as the termination > > cause a DoS of the entire VM and also we don't have a user to > > communicate to. > > I won't say is a programmer error if you are checking a value from the network. Maybe the problem is just that the value came straight from the network while it should be validated a bit before. Then I would put a check on display_handle_stream_create. Surely in that function I won't handle the invalid value as a programmer error. You can either report the error to the server or threat like a protocol error and close the connection. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel