Re: [libvirt] [PATCH] Fixes for VNC port handling
On Wed, Jan 28, 2009 at 11:29:16AM +0000, Daniel P. Berrange wrote:
> + "device/vfb/vncdisplay");
> + if (value != NULL)
> + port = strtol(value, NULL, 0);
This isn't checking the return value of strtol, so it could have
parsed garbage from XenD's SEXPR. Prefer to use virStrToLong_i()
and initialize port back to -1 upon failure.
OK.
The general idea of the patch seems correct. A question about
the test case though - the code above is dealing with correctly
handling the 'device/vfb/vncdisplay' field in the SEXPR, but
your test SEXPR data doesn't have a 'vncdisplay' field:
I screwed up the test. It was working on a live system, but of course
there's no xenstore in the test suite, so it wasn't testing what I
thought it was. I've modified the test to specify a vncdisplay.
> + (uuid 09666ad1-0c94-d79c-1439-99e05394ee51)
> + (location localhost:5900)
And I've not seen this 'location' field before - guess that's
something new in XenD we've not handled before.
Apparently so. However, handling it is strictly pointless: like
vncdisplay, it's only useful for static test cases. In all other cases
we use the authoritative xenstore results...
regards
john