PVS-Studio vs AMT SDK

AndreyKarpov · ‎07-29-2011

I checked theAMT SDK project using the PVS-Studio static code analyzer. I just glanced through the code but managed to find a few obviously odd fragments. Below I will cite the analyzer-generated messages I have studied and the corresponding code fragments. I hope this will help to improve the project a bit. You may review other odd fragments by downloading PVS-Studio from here.

I can also give you a registration key for some time. You are welcome to ask questions here: feedback.

----------------------
V501 There are identical sub-expressions to the left and to the right of the '&&' operator: name && uri && name OpenWsmanLib wsman-xml.c 156

static char *make_qname(WsXmlNodeH node, const char *uri, const char *name)
{
  char *buf = NULL;
  if (name && uri && name) {
  ...
}

Probably need:
if (node && uri && name) {
----------------------
V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 1. Present: 2. Discovery discoverysample.cpp 168

bool ParseArgs(int argc, char **argv)
{
  ...
  printf("\\nMust specify a file name when using '-report'.\\n", argv[currArg++]);
  ...
}
----------------------
False 1 3562 V547 Expression '_username == ""' is always false. To compare strings you should use strcmp() function. Discovery discoverysample.cpp 184 False
...

static char* _username = "";
static char* _password = "";
static char* _host = "";
static char* _hostMask = "";

Potentional dangerous but some time is work (in all in one obj-module):
if (((_username == "") && (_password != "")) || ((_username != "") && (_password == ""))) 
----------------------
V530 The return value of function 'empty' is required to be utilized. AMTredirection mcsol.cpp 443

bool MCSOL::_createHTLink()
{
  ...
  string HTcmd = TERM_CMD;
  ...
  HTcmd.empty();
  ...
}

Need: HTcmd.clear();
----------------------
V576 Incorrect format. A different number of actual arguments is expected while calling 'printf' function. Expected: 1. Present: 2. RemoteControlSample remotecontrolsample.cpp 792

bool GetUserValues(...)
{
  ...
  printf("Error: illegal value. Aborting.\\n", tmp);
  return false;
}
----------------------
V547 Expression 'bytesRead < 0' is always false. Unsigned type value is never < 0. RemoteConsole remote.c 173

int ReadFileData(char* fileName, UINT8 data[], size_t size)
{
  size_t fileSize , bytesRead;
  ...
  bytesRead = read(fileno(fileHandle), data, (int) size);

  if (bytesRead < 0)
  {
    printf("Error while Reading file %s : %s", fileName, strerror(errno));
    bytesRead = 0;  
  }
  ...
}
----------------------
V530 The return value of function 'empty' is required to be utilized. EventLogReader cimclass.h 153

void CimClassContainer::Clear()
{
  for(unsigned int i = 0; i < vec.size(); i++)
  {
    delete vec;
    vec = NULL;
  }
  vec.empty();
}

Need: vec.clear();
----------------------
V576 Incorrect format. A different number of actual arguments is expected while calling 'sprintf' function. Expected: 9. Present: 10. ConfigurationServer configurationutils.cpp 236

void StringFromPPS(...)
{
  ...
  sprintf((char *)textPPS, "%04X-%04X-%04X-04X-%04X-%04X-%04X-%04X\\0",
    *(unsigned long *) (hexString),
    *(unsigned short *)(hexString+4),
    *(unsigned short *)(hexString+4+4),
    *(unsigned short *)(hexString+4+4+4),
    *(unsigned short *)(hexString+4+4+4+4),
    *(unsigned short *)(hexString+4+4+4+4+4),
    *(unsigned short *)(hexString+4+4+4+4+4+4),
    *(unsigned short *)(hexString+4+4+4+4+4+4+4)
    );
  ...
}

Need add %:
sprintf((char *)textPPS, "%04X-%04X-%04X- !!!%!!! 04X-%04X-%04X-%04X-%04X\\0",
----------------------
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (pid)' expression. PskGenerator pskgenerator.cpp 126
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (pps)' expression. PskGenerator pskgenerator.cpp 127

typedef char PID[pid_len+1];
typedef char PPS[pps_len+1];

static void generate(PID pid, PPS pps)
{
  memset(pid,0,sizeof(pid));
  memset(pps,0,sizeof(pps));
  ...
}

Need:
memset(pid,0,sizeof(*pid));
memset(pps,0,sizeof(*pps));
----------------------
V547 Expression 'bytesRead < 0' is always false. Unsigned type value is never < 0. ZTCLocalAgent heciwin.cpp 357

int HECIWin::_doIoctl(...)
{
  DWORD bytesRead = 0;
  ...
  if (bytesRead < 0) {
    Deinit();
  }
  ...
}
----------------------
V576 Incorrect format. A different number of actual arguments is expected while calling 'fprintf' function. Expected: 2. Present: 3. USBFile usbfile.cpp 489

int WriteXmlFile(...)
{
  ...
  fprintf(fp, "\\n", r.Pps);
  ...
}

----------------------
V559 Suspicious assignment inside the condition expression of 'if' operator: status = true. AgentPresenceAPIConsole agentpresenceapiconsole.cpp 530

bool SetCBPolicy(...)
{
  ...
  if(status = true)
  {
    PrintSuccess();
  }

  return status;
}

Need: 
if(status == true)

And here:
V559 Suspicious assignment inside the condition expression of 'if' operator: status = true. CircuitBreaker circuitbreakersample.cpp 1637
----------------------
V519 The '* state' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1049, 1050. StorageHelper storagehelper.c 1050

PT_STATUS OpenPTSession(...)
{
  ...
  *state = PT_REGISTERED;
  *state = PT_OPENED;
  ...
}
----------------------
V576 Incorrect format. A different number of actual arguments is expected while calling '_snprintf' function. Expected: 18. Present: 19. mod_pvs mod_pvs.cpp 308

void addAttribute(...)
{
  ...
  int index = _snprintf(temp, 1023, 
    "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:02x%02x:%02x%02x:%02x%02x",
    value[0],value[1],value[2],value[3],value[4],value[5],value[6],value[7],value[8],
    value[9],value[10],value[11],value[12],value[13],value[14],value[15]);
  ...
}

Need add %:
"%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x: !!!%!!!  02x%02x:%02x%02x:%02x%02x"
----------------------
V501 There are identical sub-expressions 'options->delivery_password' to the left and to the right of the '||' operator. OpenWsmanLib wsman-client.c 631

static void
wsman_set_subscribe_options(...)
{
  ...
  if(options->delivery_certificatethumbprint ||options->delivery_password ||
     options->delivery_password) {
  ...
}

Need:
if(options->delivery_certificatethumbprint || options->delivery_username ||
   options->delivery_password) {
----------------------

Judy_H_Intel · ‎08-02-2011

Hi Andrey,
Thanks for the information. We'll take it under advisment.

Cheers,
Judy