Skip to content
This repository was archived by the owner on Feb 27, 2020. It is now read-only.

Conversation

@AlexHockey
Copy link
Contributor

@AlexHockey AlexHockey commented Mar 23, 2018

This PR enhances our logging infrastructure to allow you to pass in C++ strings. This has two advantages:

  • You don't have to tediously type .c_str() everywhere.
  • You avoid bugs where you accidentally do things like TRC_DEBUG("Hello %s", get_name().c_str()); and call .c_str() on an rvalue, which results in a dangling pointer.

I've tested this by changing one of the logs in Ralf to pass in a C++ string, and checked that the log still works.

I've wrote this code and disassambled it.

std::string name() { return "Alex"; }
void test() { TRC_DEBUG("Hello %s", name()); }

The test function calls the correct template instantiation:

   0x00000000004615af <+281>:	callq  0x4649b3 <Log::write<std::string>(int, char const*, int, char const*, std::string)>

which calls the right instantiation of _loggable:

   0x00000000004649e6 <+51>:	callq  0x465d47 <Log::_loggable<char const*>(std::string const&)>

which does call c_str 🙂

   0x0000000000465d47 <+0>:	push   %rbp
   0x0000000000465d48 <+1>:	mov    %rsp,%rbp
   0x0000000000465d4b <+4>:	sub    $0x10,%rsp
   0x0000000000465d4f <+8>:	mov    %rdi,-0x8(%rbp)
   0x0000000000465d53 <+12>:	mov    0x82ac86(%rip),%rax        # 0xc909e0 <__gcov0._ZN3Log9_loggableIPKcEES2_RKSs>
   0x0000000000465d5a <+19>:	add    $0x1,%rax
   0x0000000000465d5e <+23>:	mov    %rax,0x82ac7b(%rip)        # 0xc909e0 <__gcov0._ZN3Log9_loggableIPKcEES2_RKSs>
   0x0000000000465d65 <+30>:	mov    -0x8(%rbp),%rax
   0x0000000000465d69 <+34>:	mov    %rax,%rdi
   0x0000000000465d6c <+37>:	callq  0x4133d0 <_ZNKSs5c_strEv@plt>
   0x0000000000465d71 <+42>:	mov    0x82ac70(%rip),%rdx        # 0xc909e8 <__gcov0._ZN3Log9_loggableIPKcEES2_RKSs+8>
   0x0000000000465d78 <+49>:	add    $0x1,%rdx
   0x0000000000465d7c <+53>:	mov    %rdx,0x82ac65(%rip)        # 0xc909e8 <__gcov0._ZN3Log9_loggableIPKcEES2_RKSs+8>
   0x0000000000465d83 <+60>:	leaveq 
   0x0000000000465d84 <+61>:	retq   

This also confirms that the rvalue that is returned by name() is converted to an lvalue by the time that the function that actually does the logging is called.

I was also worried about binary sizes. I checked building sprout with and without this change. The sprout executable was 79MB before and 80MB after, so I'm not too worried.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants