Re: [Libvirt-cim] [PATCH 2 of 4] Add get_vnc_sessions() to determine all of the VNC ports available or in use
KR> +static size_t get_vnc_hash_key(unsigned int key)
KR> +{
KR> + return key % VNC_PORT_MIN;
KR> +}
What happens if I set my guest to VNC port 500?
KR> +static CMPIStatus port_to_str(unsigned int port,
KR> + char **port_str)
KR> +{
KR> + CMPIStatus s = {CMPI_RC_OK, NULL};
KR> +
KR> + if (asprintf(port_str, "%" PRIu16, port) == -1) {
KR> + cu_statusf(_BROKER, &s,
KR> + CMPI_RC_ERR_FAILED,
KR> + "Unable to determine session port");
KR> + }
KR> +
KR> + return s;
KR> +}
Hmm, I don't like the idea of alloc'ing strings to store ports.
They're just integers after all :)
KR> +static CMPIStatus port_convert(unsigned int port,
KR> + char *in_str,
KR> + int *out_port)
KR> +{
KR> + CMPIStatus s = {CMPI_RC_OK, NULL};
KR> + char *str = NULL;
KR> +
KR> + if (in_str == NULL) {
KR> + s = port_to_str(port, &str);
KR> + if (s.rc != CMPI_RC_OK)
KR> + goto out;
KR> + } else
KR> + str = strdup(in_str);
KR> +
KR> + *out_port = strtol(str, NULL, 0);
I think sscanf() is always a better choice.
KR> +static CMPIStatus get_vnc_sessions(struct vnc_ports *vnc_hash[])
KR> +{
KR> + CMPIStatus s = {CMPI_RC_OK, NULL};
KR> + const char *path = PROC_TCP;
KR> + unsigned int lport = 0;
KR> + unsigned int rport = 0;
KR> + FILE *tcp_info;
KR> + char *line = NULL;
KR> + size_t len = 0;
KR> + char *remote_port;
KR> + int local_port;
KR> + int index;
KR> + int val;
KR> + int ret;
KR> +
KR> + tcp_info = fopen(path, "r");
KR> + if (tcp_info== NULL) {
KR> + cu_statusf(_BROKER, &s,
KR> + CMPI_RC_ERR_FAILED,
KR> + "Failed to open %s: %m", tcp_info);
KR> + goto out;
KR> + }
KR> +
KR> + if (getline(&line, &len, tcp_info) == -1) {
KR> + cu_statusf(_BROKER, &s,
KR> + CMPI_RC_ERR_FAILED,
KR> + "Failed to read from %s", tcp_info);
KR> + goto out;
KR> + }
KR> +
KR> + while (getline(&line, &len, tcp_info) > 0) {
KR> + ret = sscanf(line, "%d: %*[^:]:%X %*[^:]:%X", &val,
&lport,
KR> + &rport);
KR> + if (ret != 3) {
KR> + cu_statusf(_BROKER, &s,
KR> + CMPI_RC_ERR_FAILED,
KR> + "Unable to determine active
sessions");
KR> + goto out;
KR> + }
KR> +
KR> + s = port_convert(lport, NULL, &local_port);
KR> + if (s.rc != CMPI_RC_OK)
KR> + goto out;
KR> +
KR> + if ((local_port < VNC_PORT_MIN) || (local_port >
VNC_PORT_MAX))
KR> + continue;
Ah, this is good that you check the range to avoid the crash :)
However, do we want to support guests with non-standard ports? Seems
like we do, so maybe we do need some collision resolution in the hash
logic after all.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com