[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_bloc
From: |
Jes Sorensen |
Subject: |
[Qemu-devel] Re: [PATCH 05/14] Remove unused argument for check_for_block_signature() |
Date: |
Mon, 30 Aug 2010 20:19:14 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.8) Gecko/20100806 Fedora/3.1.2-1.fc13 Lightning/1.0b2pre Thunderbird/3.1.2 |
On 08/30/10 18:40, Paolo Bonzini wrote:
> On 08/30/2010 06:16 PM, Anthony Liguori wrote:
>> This is why this type of warning sucks. Passing BlockDriverState is a
>> matter of readability because these are roughly methods. Just because
>> 'this' isn't used right now, doesn't mean that it should not be a method.
>
> On the contrary, to me this is acceptable (or even "a good thing")
> because a patch introducing the first use of a so-far-unused argument
> deserves a more careful review. In fact, if we were using C++,
> check_for_block_signature should have been static.
>
> The cases where the "this" argument is unused in a method should stand
> out as possible bugs, as is the case with the parse errors in the JSON
> parser (which _is_ a bug, as the caller cannot intercept error messages
> right now). check_for_block_signature is not one of these.
I totally agree on this. The problem with having such arguments passed
in is that you never know if they were used in the past and it was
forgotten when the code using them was removed, or if it's new code, in
which case they do deserve the extra scrutiny.
Cheers,
Jes
[Qemu-devel] [PATCH 11/14] Remove unused function arguments, Jes . Sorensen, 2010/08/30
[Qemu-devel] [PATCH 10/14] Zero json struct with memset() instea of = {} to keep compiler happy., Jes . Sorensen, 2010/08/30
[Qemu-devel] [PATCH 09/14] Remove unused arguments for add_aio_request() and free_aio_req(), Jes . Sorensen, 2010/08/30
[Qemu-devel] [PATCH 01/14] Remove unused argument for nbd_client(), Jes . Sorensen, 2010/08/30
[Qemu-devel] [PATCH 03/14] Fix repeated typo: was "end if list" instead of "end of list", Jes . Sorensen, 2010/08/30
[Qemu-devel] [PATCH 12/14] size_t is unsigned, change to ssize_t to handle errors from tight_compress_data(), Jes . Sorensen, 2010/08/30
[Qemu-devel] [PATCH 07/14] Remove unused argument for get_whole_cluster(), Jes . Sorensen, 2010/08/30