Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-18 Thread Philippe Mathieu-Daudé 

On 08/18/2017 02:32 AM, Markus Armbruster wrote:

Philippe Mathieu-Daudé  writes:


On 08/17/2017 02:55 PM, Alistair Francis wrote:

On 15/08/2017 09:30, Markus Armbruster wrote:

The stupid fix is to repeat libraries until the link succeeds:

  test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a


[...]


Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
doesn't work for me, though.

The smart solution is not to have .a reference each other.


Nah, I think we should teach those new kids on the block about -lX11
instead. :)


This sounds scary...




Paolo, what do you think?


Another possibility is to just merge the two static libraries into one.


Sounds good to me!


I feel like I have opened a can of worms.


you are good at it! IIRC it all started with a 1-line change in
tcp_chr_wait_connected() more than 2 months ago :)



I can try and combine libqemustub.a into libqemuutil.a is that the
solution? I just want to make sure before I start this.


IMHO your series is OK like this, add a "TODO remove once
libqemuutil.a circular dep is resolved" comment in the Makefile is
enough, and let this issue for another time.


I disagree.

If merging the two .a is beyond your reach (I hope it isn't), then the
spot to mess up is this one:

 # TODO bla bla explain bla
 test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a



I was talking about the first patch:

+tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o 
$(test-qom-obj-y)


not merging circularly :)

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-18 Thread Alistair Francis 
On Thu, Aug 17, 2017 at 10:32 PM, Markus Armbruster  wrote:
> Philippe Mathieu-Daudé  writes:
>
>> On 08/17/2017 02:55 PM, Alistair Francis wrote:
> On 15/08/2017 09:30, Markus Armbruster wrote:
>> The stupid fix is to repeat libraries until the link succeeds:
>>
>>  test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
>>
>> [...]
>>
>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
>> doesn't work for me, though.
>>
>> The smart solution is not to have .a reference each other.
>
> Nah, I think we should teach those new kids on the block about -lX11
> instead. :)
>>>
>>> This sounds scary...
>>>
>
>> Paolo, what do you think?
>
> Another possibility is to just merge the two static libraries into one.

 Sounds good to me!
>>>
>>> I feel like I have opened a can of worms.
>>
>> you are good at it! IIRC it all started with a 1-line change in
>> tcp_chr_wait_connected() more than 2 months ago :)
>>
>>>
>>> I can try and combine libqemustub.a into libqemuutil.a is that the
>>> solution? I just want to make sure before I start this.
>>
>> IMHO your series is OK like this, add a "TODO remove once
>> libqemuutil.a circular dep is resolved" comment in the Makefile is
>> enough, and let this issue for another time.
>
> I disagree.
>
> If merging the two .a is beyond your reach (I hope it isn't), then the
> spot to mess up is this one:

This actually seems pretty easy to do.

I'm going to split this patch and the Makefile changes into a separate
series and send those so I don't end up spamming everyone with the
earlier patches in the series. Then after 2.10 I can combine them all
and send the full series.

Thanks,
Alistair

>
> # TODO bla bla explain bla
> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
>

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-17 Thread Markus Armbruster 
Philippe Mathieu-Daudé  writes:

> On 08/17/2017 02:55 PM, Alistair Francis wrote:
 On 15/08/2017 09:30, Markus Armbruster wrote:
> The stupid fix is to repeat libraries until the link succeeds:
>
>  test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
>
> [...]
>
> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
> doesn't work for me, though.
>
> The smart solution is not to have .a reference each other.

 Nah, I think we should teach those new kids on the block about -lX11
 instead. :)
>>
>> This sounds scary...
>>

> Paolo, what do you think?

 Another possibility is to just merge the two static libraries into one.
>>>
>>> Sounds good to me!
>>
>> I feel like I have opened a can of worms.
>
> you are good at it! IIRC it all started with a 1-line change in
> tcp_chr_wait_connected() more than 2 months ago :)
>
>>
>> I can try and combine libqemustub.a into libqemuutil.a is that the
>> solution? I just want to make sure before I start this.
>
> IMHO your series is OK like this, add a "TODO remove once
> libqemuutil.a circular dep is resolved" comment in the Makefile is
> enough, and let this issue for another time.

I disagree.

If merging the two .a is beyond your reach (I hope it isn't), then the
spot to mess up is this one:

# TODO bla bla explain bla
test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-17 Thread Philippe Mathieu-Daudé 

On 08/17/2017 02:55 PM, Alistair Francis wrote:

On 15/08/2017 09:30, Markus Armbruster wrote:

The stupid fix is to repeat libraries until the link succeeds:

 test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a


[...]


Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
doesn't work for me, though.

The smart solution is not to have .a reference each other.


Nah, I think we should teach those new kids on the block about -lX11
instead. :)


This sounds scary...




Paolo, what do you think?


Another possibility is to just merge the two static libraries into one.


Sounds good to me!


I feel like I have opened a can of worms.


you are good at it! IIRC it all started with a 1-line change in 
tcp_chr_wait_connected() more than 2 months ago :)




I can try and combine libqemustub.a into libqemuutil.a is that the
solution? I just want to make sure before I start this.


IMHO your series is OK like this, add a "TODO remove once libqemuutil.a 
circular dep is resolved" comment in the Makefile is enough, and let 
this issue for another time.


Regards,

Phil.

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-17 Thread Philippe Mathieu-Daudé 

On 08/17/2017 02:02 PM, Markus Armbruster wrote:

Paolo Bonzini  writes:

On 15/08/2017 09:30, Markus Armbruster wrote:

The stupid fix is to repeat libraries until the link succeeds:

 test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a

You may have seen this with -lX11 if you're old enough.


[...]


The smart solution is not to have .a reference each other.


Nah, I think we should teach those new kids on the block about -lX11
instead. :)


haha




Paolo, what do you think?


Another possibility is to just merge the two static libraries into one.


Sounds good to me!


it makes sens to only keep libqemuutil.a with stub included.

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-17 Thread Alistair Francis 
On Thu, Aug 17, 2017 at 10:02 AM, Markus Armbruster  wrote:
> Paolo Bonzini  writes:
>
>> On 15/08/2017 09:30, Markus Armbruster wrote:
>>> The stupid fix is to repeat libraries until the link succeeds:
>>>
>>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
>>>
>>> You may have seen this with -lX11 if you're old enough.
>>>
>>> ld(1) suggests the linker can do it for us:
>>>
>>> -( archives -)
>>> --start-group archives --end-group
>>> The archives should be a list of archive files.  They may be either
>>> explicit file names, or -l options.
>>>
>>> The specified archives are searched repeatedly until no new
>>> undefined references are created.  Normally, an archive is searched
>>> only once in the order that it is specified on the command line.
>>> If a symbol in that archive is needed to resolve an undefined
>>> symbol referred to by an object in an archive that appears later on
>>> the command line, the linker would not be able to resolve that
>>> reference.  By grouping the archives, they all be searched
>>> repeatedly until all possible references are resolved.
>>>
>>> Using this option has a significant performance cost.  It is best
>>> to use it only when there are unavoidable circular references
>>> between two or more archives.
>>>
>>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
>>> doesn't work for me, though.
>>>
>>> The smart solution is not to have .a reference each other.
>>
>> Nah, I think we should teach those new kids on the block about -lX11
>> instead. :)

This sounds scary...

>>
>>> Paolo, what do you think?
>>
>> Another possibility is to just merge the two static libraries into one.
>
> Sounds good to me!

I feel like I have opened a can of worms.

I can try and combine libqemustub.a into libqemuutil.a is that the
solution? I just want to make sure before I start this.

Thanks,
Alistair

>

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-17 Thread Markus Armbruster 
Paolo Bonzini  writes:

> On 15/08/2017 09:30, Markus Armbruster wrote:
>> The stupid fix is to repeat libraries until the link succeeds:
>> 
>> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
>> 
>> You may have seen this with -lX11 if you're old enough.
>> 
>> ld(1) suggests the linker can do it for us:
>> 
>> -( archives -)
>> --start-group archives --end-group
>> The archives should be a list of archive files.  They may be either
>> explicit file names, or -l options.
>> 
>> The specified archives are searched repeatedly until no new
>> undefined references are created.  Normally, an archive is searched
>> only once in the order that it is specified on the command line.
>> If a symbol in that archive is needed to resolve an undefined
>> symbol referred to by an object in an archive that appears later on
>> the command line, the linker would not be able to resolve that
>> reference.  By grouping the archives, they all be searched
>> repeatedly until all possible references are resolved.
>> 
>> Using this option has a significant performance cost.  It is best
>> to use it only when there are unavoidable circular references
>> between two or more archives.
>> 
>> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
>> doesn't work for me, though.
>> 
>> The smart solution is not to have .a reference each other.
>
> Nah, I think we should teach those new kids on the block about -lX11
> instead. :)
>
>> Paolo, what do you think?
>
> Another possibility is to just merge the two static libraries into one.

Sounds good to me!

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-17 Thread Paolo Bonzini 
On 15/08/2017 09:30, Markus Armbruster wrote:
> The stupid fix is to repeat libraries until the link succeeds:
> 
> test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a
> 
> You may have seen this with -lX11 if you're old enough.
> 
> ld(1) suggests the linker can do it for us:
> 
> -( archives -)
> --start-group archives --end-group
> The archives should be a list of archive files.  They may be either
> explicit file names, or -l options.
> 
> The specified archives are searched repeatedly until no new
> undefined references are created.  Normally, an archive is searched
> only once in the order that it is specified on the command line.
> If a symbol in that archive is needed to resolve an undefined
> symbol referred to by an object in an archive that appears later on
> the command line, the linker would not be able to resolve that
> reference.  By grouping the archives, they all be searched
> repeatedly until all possible references are resolved.
> 
> Using this option has a significant performance cost.  It is best
> to use it only when there are unavoidable circular references
> between two or more archives.
> 
> Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
> doesn't work for me, though.
> 
> The smart solution is not to have .a reference each other.

Nah, I think we should teach those new kids on the block about -lX11
instead. :)

> Paolo, what do you think?

Another possibility is to just merge the two static libraries into one.

Paolo

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-15 Thread Markus Armbruster 
Paolo, there's a Make puzzle for you below.

Alistair Francis  writes:

> On Mon, Aug 14, 2017 at 6:34 AM, Markus Armbruster  wrote:
>> PATCH 3/5 has the exact same subject.  Why are the two separate?
>
> You are right, that is a mess.
>
> This one doesn't check for newlines at the end while the earlier one
> checked for and removed new lines.
>
>>
>> Alistair Francis  writes:
[...]
>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>> index 7af278db55..4886caf565 100644
>>> --- a/tests/Makefile.include
>>> +++ b/tests/Makefile.include
>>> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): 
>>> tests/test-thread-pool.o $(test-block-obj-y)
>>>  tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
>>>  tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
>>> $(test-crypto-obj-y)
>>>  tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
>>> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
>>> migration/page_cache.o $(test-util-obj-y)
>>> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
>>> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
>>> migration/page_cache.o $(test-qom-obj-y)
>>> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o 
>>> $(test-qom-obj-y)
>>
>> No.  What symbols exactly is the linker missing?
>
> Without the change, this is the error I see when running make check:
>
>   CC  tests/test-x86-cpuid.o
>   LINKtests/test-x86-cpuid
>   GTESTER tests/test-x86-cpuid
>   CC  tests/test-xbzrle.o
>   LINKtests/test-xbzrle
> libqemustub.a(monitor.o): In function `monitor_get_fd':
> /scratch/alistai/master-qemu/stubs/monitor.c:10: undefined reference
> to `error_setg_internal'
> collect2: error: ld returned 1 exit status
> /scratch/alistai/master-qemu/rules.mak:121: recipe for target
> 'tests/test-xbzrle' failed
> make: *** [tests/test-xbzrle] Error 1

The root cause is "obvious" ;-P

The linker searches each .a just once.  We link with

test-util-obj-y = libqemuutil.a libqemustub.a

i.e. the linker first pulls whatever it needs out of libqemuutil.a, then
moves on to pull whatever it now needs out of libqemustub.a.  If
libqemustub.a needs something from libqemuutil.a that hasn't been pulled
already, linking fails.

The linker explains its doings (--print-map):

Archive member included to satisfy reference by file (symbol)

libqemuutil.a(cutils.o)   tests/test-xbzrle.o (uleb128_encode_small)
libqemuutil.a(qemu-error.o)   libqemuutil.a(cutils.o) (warn_report)
libqemustub.a(error-printf.o)
  libqemuutil.a(qemu-error.o) (error_vprintf)
libqemustub.a(monitor.o)  libqemuutil.a(qemu-error.o) (cur_mon)

Let me explain the linker's explanation:

test-xbzrle.o pulls in libqemuutil.a(cutils.o) for
uleb128_encode_small

libqemuutil.a(cutils.o) pulls in libqemuutil.a(qemu-error.o) for
warn_report

libqemuutil.a(qemu-error.o) pulls in libqemustub.a(error_printf.o)
and libqemustub.a(monitor.o) for error_vprintf and cur_mon

libqemuutil(monitor.o) references error_setg_internal, which can't
be resolved.

The stupid fix is to repeat libraries until the link succeeds:

test-util-obj-y = libqemuutil.a libqemustub.a libqemuutil.a

You may have seen this with -lX11 if you're old enough.

ld(1) suggests the linker can do it for us:

-( archives -)
--start-group archives --end-group
The archives should be a list of archive files.  They may be either
explicit file names, or -l options.

The specified archives are searched repeatedly until no new
undefined references are created.  Normally, an archive is searched
only once in the order that it is specified on the command line.
If a symbol in that archive is needed to resolve an undefined
symbol referred to by an object in an archive that appears later on
the command line, the linker would not be able to resolve that
reference.  By grouping the archives, they all be searched
repeatedly until all possible references are resolved.

Using this option has a significant performance cost.  It is best
to use it only when there are unavoidable circular references
between two or more archives.

Sticking '-Wp,-(' and '-Wp,-)' into the command line I get from make V=1
doesn't work for me, though.

The smart solution is not to have .a reference each other.

Paolo, what do you think?

> If only the xbzrle change is made then I see this:
>
>   LINKtests/test-xbzrle
>   GTESTER tests/test-xbzrle
>   CC  tests/test-vmstate.o
>   LINKtests/test-vmstate
>   GTESTER tests/test-vmstate
>   CC  tests/test-cutils.o
>   LINKtests/test-cutils
> util/cutils.o: In function `parse_debug_env':
> /scratch/alistai/master-qemu/util/cutils.c:605: undefined reference to
>

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-14 Thread Alistair Francis 
On Mon, Aug 14, 2017 at 6:34 AM, Markus Armbruster  wrote:
> PATCH 3/5 has the exact same subject.  Why are the two separate?

You are right, that is a mess.

This one doesn't check for newlines at the end while the earlier one
checked for and removed new lines.

>
> Alistair Francis  writes:
>
>> Convert any remaining uses of fprintf(stderr, "warning:"...
>> to use warn_report() instead. This helps standardise on a single
>> method of printing warnings to the user.
>>
>> All of the warnings were changed using this command:
>>   find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] 
>> |warn_report("|Ig' {} +
>>
>> The #include lines and chagnes to the test Makefile were manually
>
> changes

Fixed.

>
>> updated to allow the code to compile.
>>
>> Signed-off-by: Alistair Francis 
>> ---
>>
>>  tests/Makefile.include | 4 ++--
>>  util/cutils.c  | 3 ++-
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 7af278db55..4886caf565 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): 
>> tests/test-thread-pool.o $(test-block-obj-y)
>>  tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
>>  tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
>> $(test-crypto-obj-y)
>>  tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
>> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
>> migration/page_cache.o $(test-util-obj-y)
>> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
>> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
>> migration/page_cache.o $(test-qom-obj-y)
>> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o 
>> $(test-qom-obj-y)
>
> No.  What symbols exactly is the linker missing?

Without the change, this is the error I see when running make check:

  CC  tests/test-x86-cpuid.o
  LINKtests/test-x86-cpuid
  GTESTER tests/test-x86-cpuid
  CC  tests/test-xbzrle.o
  LINKtests/test-xbzrle
libqemustub.a(monitor.o): In function `monitor_get_fd':
/scratch/alistai/master-qemu/stubs/monitor.c:10: undefined reference
to `error_setg_internal'
collect2: error: ld returned 1 exit status
/scratch/alistai/master-qemu/rules.mak:121: recipe for target
'tests/test-xbzrle' failed
make: *** [tests/test-xbzrle] Error 1

If only the xbzrle change is made then I see this:

  LINKtests/test-xbzrle
  GTESTER tests/test-xbzrle
  CC  tests/test-vmstate.o
  LINKtests/test-vmstate
  GTESTER tests/test-vmstate
  CC  tests/test-cutils.o
  LINKtests/test-cutils
util/cutils.o: In function `parse_debug_env':
/scratch/alistai/master-qemu/util/cutils.c:605: undefined reference to
`warn_report'
collect2: error: ld returned 1 exit status
/scratch/alistai/master-qemu/rules.mak:121: recipe for target
'tests/test-cutils' failed
make: *** [tests/test-cutils] Error 1

Thanks,
Alistair


>
>>  tests/test-int128$(EXESUF): tests/test-int128.o
>>  tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
>>  tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
>> diff --git a/util/cutils.c b/util/cutils.c
>> index 1534682083..b33ede83d1 100644
>> --- a/util/cutils.c
>> +++ b/util/cutils.c
>> @@ -30,6 +30,7 @@
>>  #include "qemu/iov.h"
>>  #include "net/net.h"
>>  #include "qemu/cutils.h"
>> +#include "qemu/error-report.h"
>>
>>  void strpadcpy(char *buf, int buf_size, const char *str, char pad)
>>  {
>> @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int 
>> initial)
>>  return initial;
>>  }
>>  if (debug < 0 || debug > max || errno != 0) {
>> -fprintf(stderr, "warning: %s not in [0, %d]", name, max);
>> +warn_report("%s not in [0, %d]", name, max);
>>  return initial;
>>  }
>>  return debug;

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-14 Thread Markus Armbruster 
PATCH 3/5 has the exact same subject.  Why are the two separate?

Alistair Francis  writes:

> Convert any remaining uses of fprintf(stderr, "warning:"...
> to use warn_report() instead. This helps standardise on a single
> method of printing warnings to the user.
>
> All of the warnings were changed using this command:
>   find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] 
> |warn_report("|Ig' {} +
>
> The #include lines and chagnes to the test Makefile were manually

changes

> updated to allow the code to compile.
>
> Signed-off-by: Alistair Francis 
> ---
>
>  tests/Makefile.include | 4 ++--
>  util/cutils.c  | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 7af278db55..4886caf565 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o 
> $(test-block-obj-y)
>  tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
>  tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
> $(test-crypto-obj-y)
>  tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
> migration/page_cache.o $(test-util-obj-y)
> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
> migration/page_cache.o $(test-qom-obj-y)
> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o 
> $(test-qom-obj-y)

No.  What symbols exactly is the linker missing?

>  tests/test-int128$(EXESUF): tests/test-int128.o
>  tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
>  tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
> diff --git a/util/cutils.c b/util/cutils.c
> index 1534682083..b33ede83d1 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -30,6 +30,7 @@
>  #include "qemu/iov.h"
>  #include "net/net.h"
>  #include "qemu/cutils.h"
> +#include "qemu/error-report.h"
>  
>  void strpadcpy(char *buf, int buf_size, const char *str, char pad)
>  {
> @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int 
> initial)
>  return initial;
>  }
>  if (debug < 0 || debug > max || errno != 0) {
> -fprintf(stderr, "warning: %s not in [0, %d]", name, max);
> +warn_report("%s not in [0, %d]", name, max);
>  return initial;
>  }
>  return debug;

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-08-03 Thread Alistair Francis 
On Fri, Jul 28, 2017 at 4:57 PM, Philippe Mathieu-Daudé  wrote:
> Hi Alistair,
>
> On 07/28/2017 07:16 PM, Alistair Francis wrote:
>>
>> Convert any remaining uses of fprintf(stderr, "warning:"...
>> to use warn_report() instead. This helps standardise on a single
>> method of printing warnings to the user.
>>
>> All of the warnings were changed using this command:
>>find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:]
>> |warn_report("|Ig' {} +
>>
>> The #include lines and chagnes to the test Makefile were manually
>> updated to allow the code to compile.
>>
>> Signed-off-by: Alistair Francis 
>> ---
>>
>>   tests/Makefile.include | 4 ++--
>>   util/cutils.c  | 3 ++-
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 7af278db55..4886caf565 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF):
>> tests/test-thread-pool.o $(test-block-obj-y)
>>   tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
>>   tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y)
>> $(test-crypto-obj-y)
>>   tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
>> -tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o
>> migration/page_cache.o $(test-util-obj-y)
>> +tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o
>> migration/page_cache.o $(test-qom-obj-y)
>
>
> I don't understand what was not working in the previous line.
>
>> -tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
>> +tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
>> $(test-qom-obj-y)
>
>
> Here adding $(util-obj-y) should be enough.
>
> But I did not test it :P

I didn't understand it either. It is possible I was doing something
really wrong, but I couldn't get it to work otherwise.

Thanks,
Alistair

>
> Regards,
>
> Phil.
>
>
>>   tests/test-int128$(EXESUF): tests/test-int128.o
>>   tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
>>   tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
>> diff --git a/util/cutils.c b/util/cutils.c
>> index 1534682083..b33ede83d1 100644
>> --- a/util/cutils.c
>> +++ b/util/cutils.c
>> @@ -30,6 +30,7 @@
>>   #include "qemu/iov.h"
>>   #include "net/net.h"
>>   #include "qemu/cutils.h"
>> +#include "qemu/error-report.h"
>> void strpadcpy(char *buf, int buf_size, const char *str, char pad)
>>   {
>> @@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int
>> initial)
>>   return initial;
>>   }
>>   if (debug < 0 || debug > max || errno != 0) {
>> -fprintf(stderr, "warning: %s not in [0, %d]", name, max);
>> +warn_report("%s not in [0, %d]", name, max);
>>   return initial;
>>   }
>>   return debug;
>>
>

Re: [Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-07-28 Thread Philippe Mathieu-Daudé 

Hi Alistair,

On 07/28/2017 07:16 PM, Alistair Francis wrote:

Convert any remaining uses of fprintf(stderr, "warning:"...
to use warn_report() instead. This helps standardise on a single
method of printing warnings to the user.

All of the warnings were changed using this command:
   find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' 
{} +

The #include lines and chagnes to the test Makefile were manually
updated to allow the code to compile.

Signed-off-by: Alistair Francis 
---

  tests/Makefile.include | 4 ++--
  util/cutils.c  | 3 ++-
  2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7af278db55..4886caf565 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o 
$(test-block-obj-y)
  tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
  tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
$(test-crypto-obj-y)
  tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
-tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
migration/page_cache.o $(test-util-obj-y)
+tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
migration/page_cache.o $(test-qom-obj-y)


I don't understand what was not working in the previous line.


-tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
+tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y)


Here adding $(util-obj-y) should be enough.

But I did not test it :P

Regards,

Phil.


  tests/test-int128$(EXESUF): tests/test-int128.o
  tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
  tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
diff --git a/util/cutils.c b/util/cutils.c
index 1534682083..b33ede83d1 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -30,6 +30,7 @@
  #include "qemu/iov.h"
  #include "net/net.h"
  #include "qemu/cutils.h"
+#include "qemu/error-report.h"
  
  void strpadcpy(char *buf, int buf_size, const char *str, char pad)

  {
@@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial)
  return initial;
  }
  if (debug < 0 || debug > max || errno != 0) {
-fprintf(stderr, "warning: %s not in [0, %d]", name, max);
+warn_report("%s not in [0, %d]", name, max);
  return initial;
  }
  return debug;

[Qemu-devel] [PATCH v2 5/5] Convert single line fprintf() to warn_report()

2017-07-28 Thread Alistair Francis 
Convert any remaining uses of fprintf(stderr, "warning:"...
to use warn_report() instead. This helps standardise on a single
method of printing warnings to the user.

All of the warnings were changed using this command:
  find ./* -type f -exec sed -i 's|fprintf(.*".*warning[,:] |warn_report("|Ig' 
{} +

The #include lines and chagnes to the test Makefile were manually
updated to allow the code to compile.

Signed-off-by: Alistair Francis 
---

 tests/Makefile.include | 4 ++--
 util/cutils.c  | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7af278db55..4886caf565 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -560,8 +560,8 @@ tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o 
$(test-block-obj-y)
 tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
 tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) 
$(test-crypto-obj-y)
 tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
-tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
migration/page_cache.o $(test-util-obj-y)
-tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
+tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
migration/page_cache.o $(test-qom-obj-y)
+tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o $(test-qom-obj-y)
 tests/test-int128$(EXESUF): tests/test-int128.o
 tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
 tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
diff --git a/util/cutils.c b/util/cutils.c
index 1534682083..b33ede83d1 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -30,6 +30,7 @@
 #include "qemu/iov.h"
 #include "net/net.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 
 void strpadcpy(char *buf, int buf_size, const char *str, char pad)
 {
@@ -601,7 +602,7 @@ int parse_debug_env(const char *name, int max, int initial)
 return initial;
 }
 if (debug < 0 || debug > max || errno != 0) {
-fprintf(stderr, "warning: %s not in [0, %d]", name, max);
+warn_report("%s not in [0, %d]", name, max);
 return initial;
 }
 return debug;
-- 
2.11.0