Re: PATCH 6/7] tpm: new 1.2 sysfs files

From: Greg KH
Date: Mon Apr 03 2006 - 17:57:25 EST


On Mon, Apr 03, 2006 at 11:42:41AM -0500, Kylene Jo Hall wrote:
> +ssize_t tpm_show_state(struct device * dev, struct device_attribute * attr,
> + char *buf)
> +{
> + u8 data[35];
> + ssize_t len;
> + char *str = buf;
> +
> + struct tpm_chip *chip = dev_get_drvdata(dev);
> + if (chip == NULL)
> + return -ENODEV;
> +
> + memcpy(data, tpm_cap, sizeof(tpm_cap));
> + data[TPM_CAP_IDX] = TPM_CAP_FLAG;
> + data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_FLAG_PERM;
> +
> + if ((len = tpm_transmit(chip, data, sizeof(data)))
> + <= TPM_ERROR_SIZE)
> + dev_dbg(chip->dev, "A TPM error (%d) occurred "
> + "attempting to determine the permanent state\n",
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> + else {
> + str +=
> + sprintf(str, "%s\n",
> + (data[TPM_GET_CAP_PERM_DISABLE_IDX] ==
> + 1) ? "Disabled" : "Enabled");
> + str +=
> + sprintf(str, "%s\n",
> + (data[TPM_GET_CAP_PERM_INACTIVE_IDX] ==
> + 1) ? "Inactive" : "Active");
> + }
> + memcpy(data, tpm_cap, sizeof(tpm_cap));
> + data[TPM_CAP_IDX] = TPM_CAP_PROP;
> + data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_PROP_OWNER;
> +
> + if ((len = tpm_transmit(chip, data, sizeof(data)))
> + <= TPM_ERROR_SIZE)
> + dev_dbg(chip->dev, "A TPM error (%d) occurred "
> + "attempting to determine the owner state\n",
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> + else
> + str +=
> + sprintf(str, "%s\n",
> + (data[TPM_GET_CAP_RET_BOOL_1_IDX] ==
> + 1) ? "Owned" : "Unowned");
> +
> + memcpy(data, tpm_cap, sizeof(tpm_cap));
> + data[TPM_CAP_IDX] = TPM_CAP_FLAG;
> + data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_FLAG_VOL;
> +
> + if ((len = tpm_transmit(chip, data, sizeof(data)))
> + <= TPM_ERROR_SIZE) {
> + dev_dbg(chip->dev, "A TPM error (%d) occurred "
> + "attempting to determine the temporary state\n",
> + be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> + goto out;
> + }
> +
> + if (data[TPM_GET_CAP_TEMP_INACTIVE_IDX] != 0)
> + str += sprintf(str, "Deactivated for this boot\n");
> +out:
> + return str - buf;
> +}
> +EXPORT_SYMBOL_GPL(tpm_show_state);

That is more than one value per file. Please make unique files for the
different capabilities. As it stands the file doesn't make too much
sense for someone reading it and not understanding that each line is a
different portion of the state.

thanks,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/