cpp:S836 The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage"
Languages
C and C++
Here we open a file and read from it. We place the file contents into a data structure. If the file input succeeds, then we read from that data structure. If it fails, then we return errno.
SQ flags this as using a garbage value. I believe this is a false positive because SQ does not know that UNIX file descriptor system calls are guaranteed to set errno upon failure. It assumes that errno could be EOK after a failure! If that happened, our payload would not be initialized before use. But it can only happen if open has a bug.
char const* m_path = "/foo/bar";
const int dcmd = 0x1234;
struct FileData
{
int magic;
};
int my_load(struct FileData & payload)
{
int const fd = open (m_path.c_str(), O_RDWR);
if (fd < 0)
{
int const retval = errno;
CP_ERROR << "open '" << m_path << "': " << strerror(retval) << endl;
assert (retval != EOK);
return retval; // Could not open -- retval is non zero!!
}
// Following call reads data and populates the payload
int dev_info = EOK;
int const retval = ::devctl(fd, dcmd, &payload, sizeof (payload), &dev_info);
close (fd);
if(retval == EOK)
{
if(dev_info == EOK)
{
return retval;
}
else
{
return dev_info;
}
}
else
{
return retval;
}
}
void test()
{
struct FileData fileData; // uninitialized
const int rc = my_load(fileData);
if (EOK == rc)
{
// SQ why u no let me access data
// fileData will be initialized if we get here
fileData >>= 1;
}
}
SQ website does not allow copy-paste, so I had to take this screenshot of the assumptions.
I suggest that when your instance upgrades to v2025.1 (the new LTA version), you see if the FP persists. Alternatively, Compiler Explorer has access to a newer version of the C/C++ analyzer with these improvements. I couldn’t get your example to parse in there, but you might have a better chance.
If in either case the FP persists, we’ll take a closer look.
Unfortunately I do not know how to run it through SQ except by checking it into my organization’s production repo. Maybe you can help me with that.
I am looking for an updated description of cpp:S836 that is smart enough to know that system functions (like open and read) indicate an error if the variable could not be initialized? This would help me make the case to upgrade our SQ instance.
In the meantime, is there a way to selectively disable this rule for this block of code where the variable is used? The only technique I know is //NOSONAR, which is overly broad.
Thanks for your prompt attention on the question.
Kevin
#include <assert.h>
#include <errno.h>
#include <iostream>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
constexpr int EOK = 0; // not defined by errno.h ??
using namespace std;
char const* m_path = "file.txt";
struct FileData
{
int magic;
};
int my_save(struct FileData const & payload)
{
int const fd = open (m_path, O_WRONLY | O_TRUNC | O_CREAT);
if (fd < 0)
{
int const retval = errno;
cerr << "my_save: open '" << m_path << "': " << strerror(retval) << endl;
assert (retval != EOK);
return retval; // Could not open -- retval is non zero!!
}
// Following call reads data and populates the payload
ssize_t n = write(fd, &payload, sizeof (payload));
close (fd);
if (n == -1)
{
int const retval = errno;
cerr << "write '" << m_path << "': " << strerror(retval) << endl;
return retval;
}
else if(n == sizeof (payload))
{
return EOK;
}
else
{
cerr << "write '" << m_path << "': file truncated" << endl;
return EIO;
}
}
int my_load(struct FileData & payload)
{
int const fd = open (m_path, O_RDONLY);
if (fd < 0)
{
int const retval = errno;
cerr << "my_load: open '" << m_path << "': " << strerror(retval) << endl;
assert (retval != EOK);
return retval; // Could not open -- retval is non zero!!
}
// Following call reads data and populates the payload
ssize_t n = read(fd, &payload, sizeof (payload));
close (fd);
if (n == -1)
{
int const retval = errno;
cerr << "read '" << m_path << "': " << strerror(retval) << endl;
return retval;
}
else if(n == sizeof (payload))
{
return EOK;
}
else
{
cerr << "read '" << m_path << "': file truncated" << endl;
return EIO;
}
}
int main()
{
{
struct FileData fileData; // uninitialized
fileData.magic = 0x1234;
const int rc = my_save(fileData);
if (rc != EOK)
return rc;
cout << "Saved " << std::hex << fileData.magic << endl;
}
struct FileData fileData; // uninitialized
const int rc = my_load(fileData);
if (EOK == rc)
{
// SQ why u no let me access data
// fileData will be initialized if we get here
fileData.magic >>= 4;
cout << "Loaded " << std::hex << fileData.magic << endl;
}
return rc;
}
