Skip to content

Commit

Permalink
Fix warnings in Linux build
Browse files Browse the repository at this point in the history
* 6 warnings are for not checking return values from system calls that indicate
errors through the return value. Fix by checking the returns properly.
* 2 warnings are for converting `NULL` to `0`; fix by using 0 where a `uint64_t`
is expected and `nullptr` otherwise.
* 2 warnings related to `memset` usage appear to be spurious, and the gcc
compiler has had similar spurious warnings before, so I'll ignore them and leave
a maintainer note.

Original warning messages below:

Core/ClientSML/src/sml_ClientAgent.cpp:1538:15: warning: ignoring return value of ‘char* getcwd(char*, size_t)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
Core/ClientSML/src/sml_ClientAgent.cpp:1708:15: warning: ignoring return value of ‘char* getcwd(char*, size_t)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
Core/SoarKernel/src/explanation_based_chunking/ebc_identity.cpp:92:94: warning: converting to non-pointer type ‘uint64_t’ {aka ‘long unsigned int’} from NULL [-Wconversion-null]
In file included from Core/SoarKernel/SoarKernel.cxx:35:
Core/SoarKernel/src/explanation_based_chunking/ebc_identity.cpp: In function ‘uint64_t get_joined_identity_chunk_inst_id(Identity*)’:
Core/SoarKernel/src/explanation_based_chunking/ebc_identity.cpp:94:94: warning: converting to non-pointer type ‘uint64_t’ {aka ‘long unsigned int’} from NULL [-Wconversion-null]
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:59:33: warning: ‘void* __builtin_memset(void*, int, long unsigned int)’ writing 16 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:59:33: warning: ‘void* __builtin_memset(void*, int, long unsigned int)’ writing 16 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
Core/CLI/src/cli_visualize.cpp:267:23: warning: ignoring return value of ‘int system(const char*)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
Core/CLI/src/cli_visualize.cpp:275:23: warning: ignoring return value of ‘int system(const char*)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
Core/CLI/src/cli_visualize.cpp:282:23: warning: ignoring return value of ‘int system(const char*)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
UnitTests/TestHarness/TestHelpers.cpp:50:14: warning: ignoring return value of ‘int chdir(const char*)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  • Loading branch information
garfieldnate committed Jan 24, 2024
1 parent 43c2ad0 commit f6d3c05
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 20 deletions.
16 changes: 13 additions & 3 deletions Core/CLI/src/cli_visualize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,22 +264,32 @@ bool CommandLineInterface::DoVisualize(const std::string* pArg1, const std::stri
lSystemCommand += lFileName;
lSystemCommand += '.';
lSystemCommand += thisAgent->visualizationManager->settings->image_type->get_value();
system(lSystemCommand.c_str());
if (system(lSystemCommand.c_str()) != 0)
{
thisAgent->visualizationManager->clear_visualization();
return SetError("Error: Could not generate visualization image because `dot` command failed. Do you have GraphViz installed?!\n");
}
}
if (thisAgent->visualizationManager->settings->launch_viewer->get_value())
{
lSystemCommand = "open ";
lSystemCommand += lFileName;
lSystemCommand += '.';
lSystemCommand += thisAgent->visualizationManager->settings->image_type->get_value();
system(lSystemCommand.c_str());
if (system(lSystemCommand.c_str()) != 0) {
thisAgent->visualizationManager->clear_visualization();
return SetError("Error: Could not generate visualization image because failed to open file generated by `dot` command.\n");
}
}
if (thisAgent->visualizationManager->settings->launch_editor->get_value())
{
lSystemCommand = "open ";
lSystemCommand += lFileName;
lSystemCommand += ".gv";
system(lSystemCommand.c_str());
if (system(lSystemCommand.c_str()) != 0)
{
return SetError("Error: Could not open .gv file for editing.\n");
}
}
if (thisAgent->visualizationManager->settings->print_gv->get_value())
{
Expand Down
12 changes: 10 additions & 2 deletions Core/ClientSML/src/sml_ClientAgent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,7 +1535,11 @@ bool Agent::SpawnDebugger(int port, const char* jarpath)
#ifdef _MSC_VER
GetCurrentDirectory(4096, buffer);
#else
getcwd(buffer, 4096);
if (getcwd(buffer, 4096) != buffer)
{
perror("getcwd");
return false;
}
#endif

std::string debuggerPath = buffer;
Expand Down Expand Up @@ -1705,7 +1709,11 @@ bool Agent::SpawnDebugger(int port, const char* jarpath)
}

char buffer[4096 + 1];
getcwd(buffer, 4096);
if (getcwd(buffer, 4096) != buffer)
{
perror("getcwd");
return false;
}

path += ":";
path += buffer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void Identity::set_operational_cond(condition* pCond, WME_Field pField)
joined_identity->touch();
}

uint64_t get_joined_identity_id(Identity* pIdentity) { if (!pIdentity) return NULL_IDENTITY_SET; else return pIdentity->get_identity(); }
Identity* get_joined_identity(Identity* pIdentity) { if (!pIdentity) return NULL; else return pIdentity->joined_identity;}
uint64_t get_joined_identity_chunk_inst_id(Identity* pIdentity) { if (!pIdentity) return NULL_IDENTITY_SET; else return pIdentity->get_clone_identity();}
uint64_t get_joined_identity_id(Identity* pIdentity) { if (!pIdentity) return NULL_IDENTIFIER_ID; else return pIdentity->get_identity(); }
Identity* get_joined_identity(Identity* pIdentity) { if (!pIdentity) return NULL_IDENTITY_SET; else return pIdentity->joined_identity;}
uint64_t get_joined_identity_chunk_inst_id(Identity* pIdentity) { if (!pIdentity) return NULL_IDENTIFIER_ID; else return pIdentity->get_clone_identity();}

4 changes: 2 additions & 2 deletions Core/SoarKernel/src/shared/memory_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,14 +271,14 @@ void* Memory_Manager::allocate_memory(size_t size, int usage_code)
if (p == NULL)
{
char msg[BUFFER_MSG_SIZE];
SNPRINTF(msg, BUFFER_MSG_SIZE, "\nmem.c: Error: Tried but failed to allocate %zu bytes of memory.\n", size);
SNPRINTF(msg, BUFFER_MSG_SIZE, "\nmemory_manager.cpp: Error: Tried but failed to allocate %zu bytes of memory.\n", size);
msg[BUFFER_MSG_SIZE - 1] = 0; /* ensure null termination */
abort_with_fatal_error_noagent(msg);
}
if (reinterpret_cast<uintptr_t>(p) & 3)
{
char msg[BUFFER_MSG_SIZE];
strncpy(msg, "\nmem.c: Error: Memory allocator returned an address that's not a multiple of 4.\n", BUFFER_MSG_SIZE);
strncpy(msg, "\nmemory_manager.cpp: Error: Memory allocator returned an address that's not a multiple of 4.\n", BUFFER_MSG_SIZE);
msg[BUFFER_MSG_SIZE - 1] = 0; /* ensure null termination */
abort_with_fatal_error_noagent(msg);
}
Expand Down
3 changes: 3 additions & 0 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ if GetOption('cc') != None:
env.Replace(CC=GetOption('cc'))
elif sys.platform == 'darwin':
env.Replace(CC='clang')

if GetOption('cxx') != None:
env.Replace(CXX=GetOption('cxx'))
elif sys.platform == 'darwin':
Expand All @@ -217,6 +218,8 @@ lnflags = []
libs = ['Soar']
if compiler == 'g++':
libs += [ 'pthread', 'dl', 'm' ]
# causes some spurious warnings; TODO: revisit and re-enable if newer compiler version fixes that
cflags.append('-Wno-stringop-overflow')
if GetOption('nosvs'):
cflags.append('-DNO_SVS')
if GetOption('defflags'):
Expand Down
24 changes: 14 additions & 10 deletions UnitTests/TestHarness/TestHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,19 @@ void setCWDToEnv()
{
if (getenv("LIBSOAR") != nullptr)
{
chdir(getenv("LIBSOAR"));
if (chdir(getenv("LIBSOAR")) != 0) {
std::cerr << "Could not change directory to getenv(\"LIBSOAR\") (" << getenv("LIBSOAR") << ")" << std::endl;
perror("chdir");
// no further action; tests might work fine anyway if the CWD is already correct
}
}
}

void printDebugInformation(std::stringstream& output, sml::Agent* agent)
{
if (!agent)
return;

// output << "============================================================" << std::endl << std::endl;
// output << "Debug Information" << std::endl << std::endl;
// output << "============================================================" << std::endl << std::endl;
Expand Down Expand Up @@ -94,37 +98,37 @@ namespace TestHelpers
#ifdef __GNUG__

std::string demangle(const char* name) {

int status = -1;

std::unique_ptr<char, void(*)(void*)> res {
abi::__cxa_demangle(name, NULL, NULL, &status),
std::free
};

return (status==0) ? res.get() : name ;
}

// VS2013 +
#elif _MSC_VER

std::string demangle(const char* name)
{
std::string result = name;
result = result.substr(result.find_last_of(" ")+1, std::string::npos);

return result;
}

// Unknown
#else

// does nothing if not g++
std::string demangle(const char* name)
{
return name;
}

#endif

};

0 comments on commit f6d3c05

Please sign in to comment.