On Tue, Dec 04, 2018 at 09:21:32AM +0100, Roberto Sassu wrote: > tpm2_get_pcr_allocation() determines if a PCR bank is allocated by checking > the mask in the TPML_PCR_SELECTION structure returned by the TPM for > TPM2_Get_Capability(). One PCR bank with algorithm set to SHA1 is always > allocated for TPM 1.x. ... > + for (j = 0; j < pcr_selection.size_of_select; j++) > + if (pcr_selection.pcr_select[j]) > + break; > + > + if (j < pcr_selection.size_of_select) { > + chip->allocated_banks[nr_alloc_banks] = hash_alg; > + nr_alloc_banks++; > + } > + Why was this needed? Can CAP_PCRS return completely unallocated banks? Kind of out-of-context for the rest of the changes. Should this be a bug fix of its own because it looks like as this is a bug fix for existing code, and not a new feature? Just asking because I don't yet fully understand this change. Anyway, I believe that you can streamline this by: /* Check that at least some of the PCRs have been allocated. This is * required because CAP_PCRS ... */ if (memchr_inv(pcr_selection.pcr_select, 0, pcr_selection.size_of_select)) nr_allocated_banks++; [yeah, comment would be awesome about CAP_PCRS. Did not finish up the comment because I don't know the answer] In addition, it would be consistent to call the local variable also nr_allocated_banks (not nr_alloc_banks). /Jarkko