Patchwork qga/channel-posix.c: Explicitly include string.h

login
register
mail settings
Submitter Peter Maydell
Date Jan. 7, 2013, 5:29 p.m.
Message ID <1357579795-11770-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/210017/
State New
Headers show

Comments

Peter Maydell - Jan. 7, 2013, 5:29 p.m.
Explicitly include string.h to avoid warnings under MacOS X/clang
about implicit declarations of strerror() and strlen().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I assume under Linux these are implicitly dragged in via one of the
other headers.

 qga/channel-posix.c | 1 +
 1 file changed, 1 insertion(+)
Stefan Weil - Jan. 7, 2013, 5:38 p.m.
Am 07.01.2013 18:29, schrieb Peter Maydell:
> Explicitly include string.h to avoid warnings under MacOS X/clang
> about implicit declarations of strerror() and strlen().
>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> I assume under Linux these are implicitly dragged in via one of the
> other headers.
>
>   qga/channel-posix.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index d4fd628..ca9e4aa 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -4,6 +4,7 @@
>   #include<unistd.h>
>   #include<fcntl.h>
>   #include<stdlib.h>
> +#include<string.h>
>   #include "qemu/osdep.h"
>   #include "qemu/sockets.h"
>   #include "qga/channel.h"
>    

Better: include qemu-common.h first and remove all
three standard includes.

- Stefan
Peter Maydell - Jan. 7, 2013, 5:50 p.m.
On 7 January 2013 17:38, Stefan Weil <sw@weilnetz.de> wrote:
> Am 07.01.2013 18:29, schrieb Peter Maydell:
>
>> Explicitly include string.h to avoid warnings under MacOS X/clang
>> about implicit declarations of strerror() and strlen().

> Better: include qemu-common.h first and remove all
> three standard includes.

This would be reversing direction from commit 4d4922c33...
I took the commit message in that commit to imply that we
shouldn't be including qemu-common.h for standalone
executables like qga.

-- PMM
Eduardo Habkost - Jan. 7, 2013, 10:12 p.m.
On Mon, Jan 07, 2013 at 05:50:14PM +0000, Peter Maydell wrote:
> On 7 January 2013 17:38, Stefan Weil <sw@weilnetz.de> wrote:
> > Am 07.01.2013 18:29, schrieb Peter Maydell:
> >
> >> Explicitly include string.h to avoid warnings under MacOS X/clang
> >> about implicit declarations of strerror() and strlen().
> 
> > Better: include qemu-common.h first and remove all
> > three standard includes.
> 
> This would be reversing direction from commit 4d4922c33...
> I took the commit message in that commit to imply that we
> shouldn't be including qemu-common.h for standalone
> executables like qga.

Including qemu-common.h from .c files is OK, although I wouldn't do that
in this specific case (I would include string.h instead).

The problem with qemu-common.h is when it is included by header files,
because qemu-common.h includes lots of other headers, easily leading to
circular header dependencies.
Stefan Weil - Jan. 8, 2013, 5:08 a.m.
> On Mon, Jan 07, 2013 at 05:50:14PM +0000, Peter Maydell wrote:
>> On 7 January 2013 17:38, Stefan Weil <sw@weilnetz.de> wrote:
>> > Am 07.01.2013 18:29, schrieb Peter Maydell:
>> >
>> >> Explicitly include string.h to avoid warnings under MacOS X/clang
>> >> about implicit declarations of strerror() and strlen().
>>
>> > Better: include qemu-common.h first and remove all
>> > three standard includes.
>>
>> This would be reversing direction from commit 4d4922c33...
>> I took the commit message in that commit to imply that we
>> shouldn't be including qemu-common.h for standalone
>> executables like qga.
>
> Including qemu-common.h from .c files is OK, although I wouldn't do that
> in this specific case (I would include string.h instead).
>
> The problem with qemu-common.h is when it is included by header files,
> because qemu-common.h includes lots of other headers, easily leading to
> circular header dependencies.
>
> --
> Eduardo
>


I just noticed that on my Linux host qemu-common.h is no longer
included indirectly (it was with earlier revisions of QEMU).

Therefore the patch is ok.

Reviewed-by: Stefan Weil <sw@weilnetz.de>
Stefan Hajnoczi - Jan. 11, 2013, 8:33 a.m.
On Mon, Jan 07, 2013 at 05:29:55PM +0000, Peter Maydell wrote:
> Explicitly include string.h to avoid warnings under MacOS X/clang
> about implicit declarations of strerror() and strlen().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I assume under Linux these are implicitly dragged in via one of the
> other headers.
> 
>  qga/channel-posix.c | 1 +
>  1 file changed, 1 insertion(+)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

Patch

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index d4fd628..ca9e4aa 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -4,6 +4,7 @@ 
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <string.h>
 #include "qemu/osdep.h"
 #include "qemu/sockets.h"
 #include "qga/channel.h"