Re: [PATCH] Raw data debugging

On 12/14/2012 01:22 AM, Amos Jeffries wrote:
> On 14/12/2012 7:30 p.m., Alex Rousskov wrote:
>> On 12/13/2012 07:04 PM, Amos Jeffries wrote:
>>> On 14/12/2012 1:54 p.m., Alex Rousskov wrote:
>>>> Hello,
>>>>
>>>> The attached patch adds Raw, an std::ostream manipulator to print
>>>> possibly non-terminated buffers and their labels/sizes.
>>>>
>>>> The Raw manipulator tries to be smart about printing buffers at various
>>>> debugging levels: Large buffers are only printed at DBG_DATA by
>>>> default.
>>>> This allows the caller to mix higher-level debug messages with dumping
>>>> of potentially large volumes of data. This smartness can be overruled
>>>> using an explicit minLevel() method call.
>>>>
>>>>
>>>> I think this manipulator can be useful in many places. The next patch I
>>>> will post illustrates such usage for helper debugging.
>>>>
>>>>
>>>> Thank you,
>>>>
>>>> Alex.
>>>
>>> How are these two different?
>>>
>>> debugs(0,9, " label: " << StringArea(buf, 10));
>>>
>>> debugs(0,9, "" << Raw("label", buf, 10));
>> There are several differences between the above lines and StringArea/Raw
>> in general:
>>
>>
>> * The StringArea example above does not even compile. Raw example does.
>> I will assume we fix that (without implementing Raw features in
>> StringArea, of course).
>>
>>
>> * StringArea is about storage. Raw is about easier and safer debugging.
>>
>>
>> * StringArea does not print the length of the buffer, which is often
>> critical in debugging. Raw does.
>>
>>
>> * StringArea should not be used for adding data/buffer debugging to
>> debugs() lines that do not use DBG_DATA. Raw is smart enough to avoid
>> dumping data in those cases, unless ALL,9 or equivalent is used. This
>> means we can have a single debugs() line that automatically gets more
>> detailed with the increased configured debugging level.
>>
>>
>> * Raw can be easily extended to supply more formatting options while
>> StringArea is not about presentation at all. For example, here how one
>> could add a limit on the maximum size of dumped contents:
>>
>> debugs(0,9, Raw("foo", buf, size).maxSize(80) << ...);
>>
>>
>> * Raw lines are shorter. There is no need to add empty string shown in
>> your example and it is easier to combine several things without spending
>> space on printing spaces:
>>
>> debugs(0,9, Raw("label", buf, 10));
>> debugs(0,9, Raw("label1", buf1, l1) << Raw("label2", buf2, l2));
>>
>>
>> * Raw does not print confusing ":" when nothing follows the label.
>>
>>
>> * We can add more formatting options (e.g., for printing binary buffers)
>> to Raw. It would be inappropriate to add that functionality to
>> StringArea.
>>
>>
>>
>>> Raw() would be elegant if it were inherited by buffer objects needing to
>>> print their data as debugs(0,9, buf).
>> I am not sure inheritance is a good model here. Raw is just a stream
>> manipulator for debugging, and the caller has to supply too many details
>> (label, minimal debugging level, etc.) for it to be a good parent class
>> for buffers. Buffers can use Raw to debug their members though!
>>
>>
>>> But otherwise this is just making
>>> a duplicate of the string+length object. We already have StringArea for
>>> C++ and lstring for C code, MemBuf in a fashion for raw buffers, and
>>> will have StringNG shortly as well.
>> Raw is about printing/debugging. All the examples you gave are about
>> storage. I agree that Raw's storage could be implemented using
>> StringArea, but I think Raw has a different overall purpose and deserves
>> to exist.
>>
>> If nobody wants Raw, I certainly do not insist on adding it, but I hope
>> the above answers will convince you that Raw is not StringArea and
>> should not be StringArea.

> Thank you. +1

Whew!

Committed to trunk as r12522, after adding another safety check to
handle nil buffer pointers with positive sizes.

Thank you,

Alex.
Received on Fri Dec 14 2012 - 16:05:41 MST