Conversation
In this header file there was no guard against including it multiple times
| std::string confPath; | ||
| for (const auto& yamlIter : yamlStep) | ||
| { | ||
| confPath = yamlIter.as<std::string>(); | ||
| } |
There was a problem hiding this comment.
why do we reinitializing that string in a loop?
Maybe just std::string confPath = yamlStep[0].as<std::string>(); is enough?
|
|
||
| std::string resExec = exec(execCommand.c_str()); | ||
|
|
||
| printf("%s", resExec.data()); |
There was a problem hiding this comment.
just
| printf("%s", resExec.data()); | |
| std::cout <<resExec; |
Or use YANET_LOG_something. Using plain printf seems strange, after all, we're using C++
| std::string command = "birdc "; | ||
| for (const auto& yamlIter : yamlStep) | ||
| { | ||
| command += yamlIter.as<std::string>() + " "; | ||
| } |
There was a problem hiding this comment.
Use std::ostringstream when building strings in a loop. Current approach is not efficient as you will gradually create more and more objects of increasing size.
| std::string command = "birdc "; | |
| for (const auto& yamlIter : yamlStep) | |
| { | |
| command += yamlIter.as<std::string>() + " "; | |
| } | |
| std::ostringstream command_stream; | |
| command_stream << "birdc "; | |
| for (const auto& yamlIter : yamlStep) | |
| { | |
| command_stream << yamlIter.as<std::string>() << ' '; | |
| } | |
| const std::string command = command_stream.str(); |
|
|
||
| auto res_birdc = exec(command.c_str()); | ||
|
|
||
| printf("birdc:\n %s\n", res_birdc.data()); |
There was a problem hiding this comment.
| printf("birdc:\n %s\n", res_birdc.data()); | |
| std::cout << "birdc:\n" << res_birdc << '\n'; |
| - bird/bird.conf | ||
| - birdc: | ||
| - show route | ||
| - sleep: 20 |
There was a problem hiding this comment.
is 20 second sleep absolutely needed? I would like to see a lesser value though if it's necessary, then feel free to ignore
|
Btw this pull request is set to merge to bird-import-proto, not |
stal76
force-pushed
the
bird-import-proto
branch
5 times, most recently
from
2416df2 to
b12ac9a
Compare
4 participants
No description provided.