[libvirt] [PATCH REPOST 04/38] virlog: Export virLogOutputPtr through header

Erik Skultety eskultet at redhat.com
Mon Jul 4 14:57:21 UTC 2016


On 10/05/16 12:29, Erik Skultety wrote:
> On 10/05/16 02:08, Cole Robinson wrote:
>> On 05/04/2016 10:30 AM, Erik Skultety wrote:
>>> It needs to be exported, since some caller might (for some reason) want to
>>> create a logging output without calling the parser which does this. Also,
>>> some methods will use virLogOutputPtr as data type for one of its arguments.
>>> ---
>>>  src/util/virlog.c | 2 --
>>>  src/util/virlog.h | 3 +++
>>>  2 files changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/util/virlog.c b/src/util/virlog.c
>>> index 812e2cd..0be1701 100644
>>> --- a/src/util/virlog.c
>>> +++ b/src/util/virlog.c
>>> @@ -106,8 +106,6 @@ struct _virLogOutput {
>>>      virLogDestination dest;
>>>      char *name;
>>>  };
>>> -typedef struct _virLogOutput virLogOutput;
>>> -typedef virLogOutput *virLogOutputPtr;
>>>  
>>>  static virLogOutputPtr *virLogOutputs;
>>>  static size_t virLogNbOutputs;
>>> diff --git a/src/util/virlog.h b/src/util/virlog.h
>>> index b5056f5..7706d22 100644
>>> --- a/src/util/virlog.h
>>> +++ b/src/util/virlog.h
>>> @@ -130,6 +130,9 @@ struct _virLogMetadata {
>>>  typedef struct _virLogMetadata virLogMetadata;
>>>  typedef struct _virLogMetadata *virLogMetadataPtr;
>>>  
>>> +typedef struct _virLogOutput virLogOutput;
>>> +typedef virLogOutput *virLogOutputPtr;
>>> +
>>>  /**
>>>   * virLogOutputFunc:
>>>   * @src: the source of the log message
>>>
>>
>> ACK, but IMO exporting it early in a separate patch without context makes it
>> hard to follow the reasoning. Better would have been to export it with the
>> first public function that needs it, looks like virLogDefineOutputs
>>
>> - Cole
>>
> 
> I tried to break all the changes into as many bits as possible, so that
> it could be reviewed more easily and I did it with the best intentions,
> but I have to admit that you're absolutely right and without any
> context, this can be very hard to follow for a reviewer. Also, for the
> sake of commit history, I think squashing this change into the commit
> where I'm introducing virLogDefineOutputs might be a better choice than
> pushing this as a standalone patch. Analogically, I'll squash 5/38 into
> the commit which introduces virLogDefineFilters.
> 
> Erik

So, since there was a gap in the ACKed patches, I moved 15-18 a bit to
front, squashed 4 and 5 into them (originally I squashed them into 10
and 11, but I didn't get an ACK on those. Anyhow exporting a pointer
type wouldn't make a difference in any of those cases...) and pushed.

Thanks,
Erik




More information about the libvir-list mailing list