Not using braces and the logging library is error prone

[andrejlevkovitch]

In the repository, for some reason, braces are not using in case when there is just one line after if or else. This is already a potential problem, because that is extremely error prone approach, especially if formatter (like clang-format) is not in use. For demonstrate how bad it is I want to show you how compiler handles one of if-else statements in the code.

Here it is:

jetson-utils/video/videoOptions.cpp

Lines 55 to 58 in d374d42

	if( prefix != NULL )
	LogInfo("%s video options:\n", prefix);
	else
	LogInfo("video options:\n");

So, according to the lines, I expect that if prefix is NULL I should see video options:\n line in the logs. But here is a problem: the if-else doesn't have braces and it uses macros-es inside. Macros is not a function, so a priori you should not make assumptions about how much lines it takes. One line macros is not always equal to just line of code for compiler. And in that particular case it leads to a bug (not fatal, but still). Let's expand that macros-es:

if (prefix != NULL)
  GenericLogMessage(Log::Info, LOG_LEVEL_PREFIX_INFO "%s video options:\n", prefix);
else
  GenericLogMessage(Log::Info, LOG_LEVEL_PREFIX_INFO "video options:\n");

GenericLogMessage is also a macros, so lets expand further:

if (prefix != NULL)
  if (Log::Info <= Log::GetLevel()) fprintf(Log::GetFile(), LOG_LEVEL_PREFIX_INFO "%s video options:\n", prefix);
else
  if (Log::Info <= Log::GetLevel()) fprintf(Log::GetFile(), LOG_LEVEL_PREFIX_INFO "video options:\n");

Hm... So, here is a question: to what if that else relates?

PS I hope that you will abandoned that error prone style and start using braces even if there is "just one line of code". There are some tools that can help with that migration, like clang-format and clang-tidy

[dusty-nv]

Hi @andrejlevkovitch, thanks for raising this and appreciate your PR's - just merged them.