On 11/11/2015 10:17 AM, Daniel P. Berrange wrote:
>> + virCommandSetOutputBuffer(cmd, &output);
>> + size_t count = -1; // Don't count table header line
>> + size_t i = 0;
>> + if (virCommandRun(cmd, NULL) < 0)
>> + goto error;
>> + while (output[i] != '\0') {
>> + if (output[i] == '\n') count++;
>> + i++;
>> + }
>> + if (VIR_ALLOC_N(*parsedOutput, count)) {
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Failed to allocate jailhouse_cell array of size %zu"), count);
>
> Again no need to report error. You need to check
>
> VIR_ALLOC_N(...) < 0
>
> too - this error check is inverted right now
I removed the report, but I don't quite understand why this check would be inverted.
VIR_ALLOC_N returns 0 on success, in which case it won't enter the if block and -1 on failure(OOM) in which case the if block will be entered.
Care to elaborate?
>> + i = 0;
>> + size_t j;
>> + while (output[i++] != '\n'); // Skip table header line
>> + for (j = 0; j < count; j++) {
>> + size_t k;
>> + for (k = 0; k <= IDLENGTH; k++) // char after number needs to be NUL for virStrToLong
>> + if (output[i+k] == ' ') {
>> + output[i+k] = '\0';
>> + break;
>> + }
>> + char c = output[i+IDLENGTH];
>> + output[i+IDLENGTH] = '\0'; // in case ID is 8 chars long, so beginning of name won't get parsed
>> + if (virStrToLong_i(output+i, NULL, 0, &(*parsedOutput)[j].id))
>> + virReportError(VIR_ERR_INTERNAL_ERROR,
>> + _("Failed to parse id to long: %s"), output+i);
>> + output[i+IDLENGTH] = c;
>> + i += IDLENGTH;
>> + if (virStrncpy((*parsedOutput)[j].name, output+i, NAMELENGTH, NAMELENGTH+1) == NULL)
>> + // should never happen
>> + goto error;
>> + (*parsedOutput)[j].name[NAMELENGTH] = '\0';
>> + for (k = 0; k < NAMELENGTH; k++)
>> + if ((*parsedOutput)[j].name[k] == ' ')
>> + break;
>> + (*parsedOutput)[j].name[k] = '\0';
>> + i += NAMELENGTH;
>> + if (STREQLEN(output+i, STATERUNNINGSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATERUNNING;
>> + else if (STREQLEN(output+i, STATESHUTDOWNSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATESHUTDOWN;
>> + else if (STREQLEN(output+i, STATEFAILEDSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATEFAILED;
>> + else if (STREQLEN(output+i, STATERUNNINGLOCKEDSTRING, STATELENGTH)) (*parsedOutput)[j].state = STATERUNNINGLOCKED;
>> + i += STATELENGTH;
>> + (*parsedOutput)[j].assignedCPUsLength = parseCPUs(output+i, &((*parsedOutput)[j].assignedCPUs));
>> + i += CPULENGTH;
>> + (*parsedOutput)[j].failedCPUsLength = parseCPUs(output+i, &((*parsedOutput)[j].failedCPUs));
>> + i += CPULENGTH;
>> + i++; // skip \n
>> + }
>
> It would be nice to include an example of the 'cell list' output to understand
> what this code is trying to parse. Then i migth be able to suggest a way
> to do this more simply.
ID Name State Assigned CPUs Failed CPUs
0 QEMU-VM running 0-3
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list