[Spice-devel] [PATCH] validate tcp port values

[Uri Lublin]

On 07/12/2012 02:31 PM, Peter Hatina wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 07/11/2012 02:03 PM, Uri Lublin wrote:
>> On 07/11/2012 02:03 PM, Peter Hatina wrote:
>>> Hi,
>>>
>>> I had a request to validate TCP port values in spice-xpi, so does
>>> this make sense for you?
>>>
>>> More info at https://bugzilla.redhat.com/show_bug.cgi?id=805602
>>>
>>> --- SpiceXPI/src/plugin/plugin.cpp |   27
>>> +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2
>>> deletions(-)
>>>
>>> diff --git a/SpiceXPI/src/plugin/plugin.cpp
>>> b/SpiceXPI/src/plugin/plugin.cpp index b7f61ba..4aac37e 100644
>>> --- a/SpiceXPI/src/plugin/plugin.cpp +++
>>> b/SpiceXPI/src/plugin/plugin.cpp @@ -581,6 +593,15 @@ void
>>> nsPluginInstance::SendStr(uint32_t id, std::string str)
>>>
>>> void nsPluginInstance::Connect() { +    const int port =
>>> portToInt(m_port); +    const int sport =
>>> portToInt(m_secure_port); +    if (port<= 0) +
>>> g_warning("invalid port: '%s'", m_port.c_str()); +    if (sport<=
>>> 0) +        g_warning("invalid secure port: '%s'",
>>> m_secure_port.c_str()); +    if (port<= 0&&   sport<= 0) +
>>> return; +
>> I think simply returning here is not good. You should also send a
>> "disconnected" (or spice client exited) notification to the
>> caller.
> Fine, the only notification, we currently have and is not fully
> supported is calling JS OnDisconnected(). Or do you have something
> specific on your mind?

I'm not sure.
The caller expects to be notified when the client is not connected.
Connect() does not return a value.

Calling OnDisconnected is what I thought is the right thing to do.
But looking at ovirt's code [1], I see it's polling on ConnectedStatus(),
   so maybe it's enough to set m_connected_status to 1.
Or do both.

[1] 
http://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=blob_plain;f=frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/uicommon/SpiceInterfaceImpl.java 
-- see connectNativelyViaXPI  below client.connect()

Regards,
     Uri.