When we trace to file, add eol of each trace message.

When we trace to file, add eol of each trace message.

BUG=webrtc:4864
TEST=Manual Test with voe_cmd_test
R=pbos@webrtc.org
TBR=kjellander@webrtc.org

The regression is introduced in #8529, Review URL: https://webrtc-codereview.appspot.com/34329004.

Committed: https://chromium.googlesource.com/external/webrtc/+/a44660960a24c5167ad4b8b15093d577dfc224b0

[braveyao1, 2015-07-24 06:46:57]

braveyao@webrtc.org changed reviewers:
+ pbos@webrtc.org

[braveyao1, 2015-07-24 06:46:57]

Hi pbos, please help to take a look!

[pbos-webrtc, 2015-07-24 08:29:11]

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
File webrtc/system_wrappers/source/trace_impl.cc (right):

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/trace_impl.cc:452: trace_message[ack_len - 1] =
'\n';
Doesn't this truncate the last character?

[pbos-webrtc, 2015-07-24 09:47:34]

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
File webrtc/system_wrappers/source/trace_impl.cc (right):

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/trace_impl.cc:405: trace_file_->Write(msg,
length);
Isn't here where you want to add the '\n'? You shouldn't add it to all logging
right?

[braveyao1, 2015-07-24 10:02:39]

Thanks for the comments. All replied. No modification needed I think. Please
re-consider.

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
File webrtc/system_wrappers/source/trace_impl.cc (right):

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/trace_impl.cc:405: trace_file_->Write(msg,
length);
On 2015/07/24 09:47:34, pbos-webrtc wrote:
> Isn't here where you want to add the '\n'? You shouldn't add it to all logging
> right?

It used to be here. But now since the input parameter is a const. So it need
some buffer copy. Not necessary I suppose.
Yes we should add new line to all the logging.

https://codereview.webrtc.org/1256773002/diff/1/webrtc/system_wrappers/source...
webrtc/system_wrappers/source/trace_impl.cc:452: trace_message[ack_len - 1] =
'\n';
On 2015/07/24 08:29:11, pbos-webrtc wrote:
> Doesn't this truncate the last character?

I was afraid so. But magically it doesn't. It's equivalence as before.

[pbos-webrtc, 2015-07-24 10:41:48]

On 2015/07/24 10:02:39, braveyao1 wrote:
> Thanks for the comments. All replied. No modification needed I think. Please
> re-consider.
> 

What about all console logging? "callback_->Print(level, trace_message,
length);" in AddMessageToList now has an additional \n, that can't be a no-op.

[braveyao1, 2015-07-27 04:16:06]

On 2015/07/24 10:41:48, pbos-webrtc wrote:
> On 2015/07/24 10:02:39, braveyao1 wrote:
> > Thanks for the comments. All replied. No modification needed I think. Please
> > re-consider.
> > 
> 
> What about all console logging? "callback_->Print(level, trace_message,
> length);" in AddMessageToList now has an additional \n, that can't be a no-op.

I did check both console logging and trace file, but still made some mistake at
that time I suppose.
You're right. Corrected it. PTAL!

[pbos-webrtc, 2015-07-27 11:53:49]

lgtm, thanks

[braveyao1, 2015-07-28 02:46:16]

Committed patchset #2 (id:20001) manually as
a44660960a24c5167ad4b8b15093d577dfc224b0 (presubmit successful).