Re: [PATCH] Raw data debugging

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,

Alex.
Received on Fri Dec 14 2012 - 06:30:45 MST