Re: [Qemu-devel] [PATCH] tpm: Fix compilation with --disable-tpm

[Juan Quintela]

Laurent Vivier <address@hidden> wrote:
> On 18/10/2017 10:33, Juan Quintela wrote:
>> Commit
>>    c37cacabf2285b0731b44c1f667781fdd4f2b658
>> 
>> broke compilation without tpm.  Just add an empty tpm_cleanup() stub.
>
> tpm_init() and tpm_config_parse() are managed in a different way: they
> are included in a "#ifdef CONFIG_TPM .. #endif" in vl.c
>
> I think you should follow the same way.

tpm is weird (TM):

in include/sysemu/tpm.h we do that in an incline function

static inline TPMVersion tpm_get_version(void)
{
#ifdef CONFIG_TPM
    Object *obj = object_resolve_path_type("", TYPE_TPM_TIS, NULL);

    if (obj) {
        return tpm_tis_get_tpm_version(obj);
    }
#endif
    return TPM_VERSION_UNSPEC;
}


tpm.c, we have an ifdef in the middle of the file

#ifdef CONFIG_TPM

#endif


vl.c, we protect tpm_* calls with CONFIG_TPM

#ifdef CONFIG_TPM
            case QEMU_OPTION_tpmdev:
                if (tpm_config_parse(qemu_find_opts("tpmdev"), optarg) < 0) {
                    exit(1);
                }
                break;
#endif


but we almost never do:

#ifdef CONFIG_TPM
   tpm_cleanup()
#endif

We normally create an stub function.

So ......

We are going to be inconsistent whatever we did.

I would have vote for

#ifdef CONFIG_TPM
void tpm_cleanup(void);
#else
static inline void tpm_cleanup(void) {}
#endif

On the header file (it was my first solution), but having CONFIG_TPM on
tpm.c sounded gross.

So ....

I think that now that the problem is spotted, I will left the choose of
the solution to the maintaner O:-)

Later, Juan.