795126 - Make iostream-based debugging DEBUG-only (signaling, mtransport)

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

[17:25]	bsmedberg	jesup: iostream has caused linkage issues in the past, but I forget why. I don't think we should be using it in production environments without more thought, but for logging-debugging code I think the rule can be "if it works on try it's ok"
[17:25]	bsmedberg	jesup: the problems would be dealing with code that threw exceptions on IO failure, mainly

We should look to make them disappear on production, and debugs needed on production move to avoid iostreams.

This assumes they don't do that already for all the cases.

Comment 1

[Ethan Hugg [:ehugg]]

In the signaling code CSFLogDebugS and CSFLogErrorS are the main things that use streams.  They could be replaced by their printf-style equivalents without the trailing S.  The stream versions were created because they were considered safer than the printf-style ones.

Comment 2

[Ethan Hugg [:ehugg]]

Attachment: Remove io streams in Signaling code for non-debug builds

Comment 3

[Ethan Hugg [:ehugg]]

Attachment: Remove io streams in Signaling code for non-debug builds

Comment 4

[Ethan Hugg [:ehugg]]

Comment on attachment 668542 [details] [diff] [review]
Remove io streams in Signaling code for non-debug builds


All usages of stream should be removed now from signaling unless DEBUG is defined.

Comment 5

[Eric Rescorla (:ekr)]

Comment on attachment 668542 [details] [diff] [review]
Remove io streams in Signaling code for non-debug builds

Review of attachment 668542 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

::: media/webrtc/signaling/src/common/browser_logging/CSFLogStream.h
@@ +55,5 @@
>  #define CSFLogDebugS(tag, message)		{ std::ostringstream _oss; _oss << message << std::endl; CSFLog( CSF_LOG_DEBUG, __FILE__ , __LINE__ , tag, _oss.str().c_str()); }
>  
> +#else // DEBUG
> +
> +#define CSFLogCriticalS(tag, message)

I would do {} to keep the syntax the same.

Comment 6

[Ethan Hugg [:ehugg]]

Comment on attachment 668542 [details] [diff] [review]
Remove io streams in Signaling code for non-debug builds


Signaling code patch pushed to Alder - https://hg.mozilla.org/projects/alder/rev/b73eec736a65