diff mbox

[kvm-unit-tests,18/32] lib: provide stdio.h

Message ID 20170421005004.137260-19-dmatlack@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Matlack April 21, 2017, 12:49 a.m. UTC
From: Peter Feiner <pfeiner@google.com>

kvm-unit-tests builds with the odd combination of system include files
but not system libraries. That is, -nostdlib is specified but
-nostdinc isn't! Problems arise when the include files don't match the
libraries, e.g., printf() in some versions of glibc linking in a
__printf_chk symbol that libcflat doesn't have. This patch fixes the
problem with printf by pointing '#include "stdio.h"' to
lib/libcflat.h, where a printf declaration that libcflat actually
implements exists, rather than letting /usr/include/stdio.h get
included. This patch does not solve the general problem of potential
mismatches between libcflat and the compiler's notion standard include
files; doing so would require rewriting stdarg.h, stdint.h, and
several others in libcflat.

Signed-off-by: Peter Feiner <pfeiner@google.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 lib/stdio.h | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 lib/stdio.h

Comments

Andrew Jones April 21, 2017, 7:16 a.m. UTC | #1
On Thu, Apr 20, 2017 at 05:49:50PM -0700, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> kvm-unit-tests builds with the odd combination of system include files
> but not system libraries. That is, -nostdlib is specified but
> -nostdinc isn't! Problems arise when the include files don't match the
> libraries, e.g., printf() in some versions of glibc linking in a
> __printf_chk symbol that libcflat doesn't have. This patch fixes the
> problem with printf by pointing '#include "stdio.h"' to
> lib/libcflat.h, where a printf declaration that libcflat actually
> implements exists, rather than letting /usr/include/stdio.h get
> included. This patch does not solve the general problem of potential
> mismatches between libcflat and the compiler's notion standard include
> files; doing so would require rewriting stdarg.h, stdint.h, and
> several others in libcflat.

Yeah, I'm not sure rewriting those would be worth being able to add
-nostdinc. When we cross compile ARM and POWER those few headers are
some of the ones provided by the cross compiler.

> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>  lib/stdio.h | 4 ++++
>  1 file changed, 4 insertions(+)
>  create mode 100644 lib/stdio.h
> 
> diff --git a/lib/stdio.h b/lib/stdio.h
> new file mode 100644
> index 000000000000..7b30db93f7f1
> --- /dev/null
> +++ b/lib/stdio.h
> @@ -0,0 +1,4 @@
> +#ifndef LIBCFLAT_STDIO_H
> +#define LIBCFLAT_STDIO_H
> +#include "libcflat.h"
> +#endif
> -- 
> 2.12.2.816.g2cccc81164-goog
>

I'd rather we move the stdio functions (just printf?) into stdio.h rather
than redirect to libcflat. We can include stdio.h from libcflat.h to
avoid updating all the unit tests for now though.

Thanks,
drew
Paolo Bonzini April 21, 2017, 8:24 a.m. UTC | #2
On 21/04/2017 09:16, Andrew Jones wrote:
>>
>> kvm-unit-tests builds with the odd combination of system include files
>> but not system libraries. That is, -nostdlib is specified but
>> -nostdinc isn't! Problems arise when the include files don't match the
>> libraries, e.g., printf() in some versions of glibc linking in a
>> __printf_chk symbol that libcflat doesn't have. This patch fixes the
>> problem with printf by pointing '#include "stdio.h"' to
>> lib/libcflat.h, where a printf declaration that libcflat actually
>> implements exists, rather than letting /usr/include/stdio.h get
>> included. This patch does not solve the general problem of potential
>> mismatches between libcflat and the compiler's notion standard include
>> files; doing so would require rewriting stdarg.h, stdint.h, and
>> several others in libcflat.
> 
> Yeah, I'm not sure rewriting those would be worth being able to add
> -nostdinc. When we cross compile ARM and POWER those few headers are
> some of the ones provided by the cross compiler.

I think -sysroot=`pwd` would keep the usual compiler include path, and
remove /usr/include.

Paolo
David Matlack April 21, 2017, 7:40 p.m. UTC | #3
On Fri, Apr 21, 2017 at 12:16 AM, Andrew Jones <drjones@redhat.com> wrote:
> On Thu, Apr 20, 2017 at 05:49:50PM -0700, David Matlack wrote:
>> From: Peter Feiner <pfeiner@google.com>
>>
>> kvm-unit-tests builds with the odd combination of system include files
>> but not system libraries. That is, -nostdlib is specified but
>> -nostdinc isn't! Problems arise when the include files don't match the
>> libraries, e.g., printf() in some versions of glibc linking in a
>> __printf_chk symbol that libcflat doesn't have. This patch fixes the
>> problem with printf by pointing '#include "stdio.h"' to
>> lib/libcflat.h, where a printf declaration that libcflat actually
>> implements exists, rather than letting /usr/include/stdio.h get
>> included. This patch does not solve the general problem of potential
>> mismatches between libcflat and the compiler's notion standard include
>> files; doing so would require rewriting stdarg.h, stdint.h, and
>> several others in libcflat.
>
> Yeah, I'm not sure rewriting those would be worth being able to add
> -nostdinc. When we cross compile ARM and POWER those few headers are
> some of the ones provided by the cross compiler.
>
>>
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> Signed-off-by: David Matlack <dmatlack@google.com>
>> ---
>>  lib/stdio.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>  create mode 100644 lib/stdio.h
>>
>> diff --git a/lib/stdio.h b/lib/stdio.h
>> new file mode 100644
>> index 000000000000..7b30db93f7f1
>> --- /dev/null
>> +++ b/lib/stdio.h
>> @@ -0,0 +1,4 @@
>> +#ifndef LIBCFLAT_STDIO_H
>> +#define LIBCFLAT_STDIO_H
>> +#include "libcflat.h"
>> +#endif
>> --
>> 2.12.2.816.g2cccc81164-goog
>>
>
> I'd rather we move the stdio functions (just printf?) into stdio.h rather
> than redirect to libcflat. We can include stdio.h from libcflat.h to
> avoid updating all the unit tests for now though.

That sounds reasonable to me. Feel free to leave this patch out when
applying if you'd like. None of the other patches in this series
actually depend on it.

>
> Thanks,
> drew
diff mbox

Patch

diff --git a/lib/stdio.h b/lib/stdio.h
new file mode 100644
index 000000000000..7b30db93f7f1
--- /dev/null
+++ b/lib/stdio.h
@@ -0,0 +1,4 @@ 
+#ifndef LIBCFLAT_STDIO_H
+#define LIBCFLAT_STDIO_H
+#include "libcflat.h"
+#endif