Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

static analyze report created with PVS-Studio #72

Closed
p-alik opened this issue Jan 30, 2017 · 7 comments
Closed

static analyze report created with PVS-Studio #72

p-alik opened this issue Jan 30, 2017 · 7 comments

Comments

@p-alik
Copy link
Collaborator

p-alik commented Jan 30, 2017

Here is a static analyze report created for gearmand at 15 October 2016 with PVS Studio

@p-alik
Copy link
Collaborator Author

p-alik commented Jan 30, 2017

a little bit of prose with regard to most in report most presented vulnerability V690

@esabol
Copy link
Collaborator

esabol commented Feb 22, 2017

Nitpick: Typo in issue title. Should be "PVS-Studio".

Seems like stuff worth fixing. Might want to break out the problem into separate issues for each type of problem: V690, V730, etc.

@p-alik p-alik changed the title static analyze report created with PVS-Stutio static analyze report created with PVS-Studio Feb 22, 2017
@p-alik
Copy link
Collaborator Author

p-alik commented Feb 22, 2017

Indeed, the issue should be separated.
I added a todo to the Project Board

@p-alik
Copy link
Collaborator Author

p-alik commented May 24, 2017

List of the issues:

  • 1.
Viva64-EM
full
54
/nfs/home/dmka/src/gearmand/libgearman/vector.hpp
error
V690
The 'gearman_vector_st' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class.
false
2
*/
struct gearman_vector_st {
  char *end;
------------
  • 2.
Viva64-EM
full
47
/nfs/home/dmka/src/gearmand/libgearman/server_options.hpp
error
V690
The 'gearman_server_options_st' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class.
false
2

struct gearman_server_options_st
{
------------
  • 3.
Viva64-EM
full
47
/nfs/home/dmka/src/gearmand/libgearman/connection.hpp
error
V690
The 'gearman_connection_st' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class.
false
2

struct gearman_connection_st
{
------------
  • 4.
Viva64-EM
full
218
/nfs/home/dmka/src/gearmand/libgearman/packet.cc
error
V547
Expression is always false. Pointer 'gearman_packet_create(universal, packet)' != NULL.
false
2
{
  if (gearman_packet_create(universal, packet) == NULL)
  {
------------
  • 5.
Viva64-EM
full
78
/nfs/home/dmka/src/gearmand/libgearman/interface/packet.hpp
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: arg, arg_size, args_buffer.
false
2

  gearman_packet_st(bool is_allocted_= false) :
    options(is_allocted_),
------------
  • 6.
Viva64-EM
full
149
/nfs/home/dmka/src/gearmand/libgearman/protocol/submit.cc
error
V576
Incorrect format. Consider checking the fourth actual argument of the 'snprintf' function. The UNSIGNED argument of memsize type is expected.
false
3
  char time_string[30];
  int length= snprintf(time_string, sizeof(time_string), "%" PRIu64, static_cast<int64_t>(when));
  args[2]= time_string;
------------
  • 7.
Viva64-EM
full
103
/nfs/home/dmka/src/gearmand/libgearman/protocol/submit.cc
error
V524
It is odd that the body of 'submit_background' function is fully equivalent to the body of 'submit' function.
false
3

gearman_return_t submit_background(gearman_universal_st& universal,
                                   gearman_packet_st& message,
93,103
------------
  • 8.
Viva64-EM
full
61
/nfs/home/dmka/src/gearmand/libhashkit/encrypt.cc
error
V542
Consider inspecting an odd type cast: 'void *' to 'bool'.
false
1
  
  return bool(kit->_key);
}
------------
  • 9.
Viva64-EM
full
68
/nfs/home/dmka/src/gearmand/libgearman-server/struct/gearmand.h
error
V519
The '_keepalive' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 67, 68.
false
2
      _keepalive= true;
      _keepalive= keepalive_;
    }
67,68
------------
  • 10.
Viva64-EM
full
187
/nfs/home/dmka/src/gearmand/libgearman-server/plugins/protocol/http/protocol.cc
error
V576
Incorrect format. Consider checking the fourth actual argument of the 'snprintf' function. The UNSIGNED integer type argument is expected.
false
3
      {
        pack_size= (size_t)snprintf((char *)send_buffer, send_buffer_size,
                                    "HTTP/1.0 %u %s\r\n"
------------
  • 11.
Viva64-EM
full
70
/nfs/home/dmka/src/gearmand/libgearman-server/struct/packet.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: arg, arg_size, args_buffer.
false
2

  gearmand_packet_st():
    magic(GEARMAN_MAGIC_TEXT),
------------
  • 12.
Viva64-EM
full
124
/nfs/home/dmka/src/gearmand/libgearman-server/struct/io.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: options, _state, send_state, recv_state, events, revents, ...
false
3

  gearmand_io_st() { }
------------
  • 13.
Viva64-EM
full
179
/nfs/home/dmka/src/gearmand/libgearman-server/struct/io.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: is_sleeping, is_exceptions, is_dead, is_noop_sent, is_cleaned_up, ret, ...
false
3

  gearman_server_con_st()
  {
------------
  • 14.
Viva64-EM
full
115
/nfs/home/dmka/src/gearmand/libgearman-server/struct/server.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: flags, state, shutdown, shutdown_graceful, proc_wakeup, proc_shutdown, ...
false
3

  gearman_server_st()
  {
------------
  • 15.
Viva64-EM
full
55
/nfs/home/dmka/src/gearmand/libgearman-server/struct/port.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: _remove_fn.
false
1

  gearmand_port_st() :
    listen_count(0),
------------
  • 16.
Viva64-EM
full
141
/nfs/home/dmka/src/gearmand/libgearman-server/struct/gearmand.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: wakeup_event.
false
1

  gearmand_st(const char *host_,
              uint32_t threads_,
------------
  • 17.
Viva64-EM
full
93
/nfs/home/dmka/src/gearmand/libgearman-server/plugins/queue/sqlite/queue.cc
error
V668
There is no sense in testing the 'exec_queue' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error.
false
2

  if (exec_queue == NULL)
  {
------------
  • 18.
Viva64-EM
full
73
/nfs/home/dmka/src/gearmand/libgearman-server/byteorder.cc
error
V524
It is odd that the body of 'htonll' function is fully equivalent to the body of 'ntohll' function.
false
3

uint64_t htonll(uint64_t value)
{
68,73
------------
  • 19.
Viva64-EM
full
162
/nfs/home/dmka/src/gearmand/libgearman-server/gearmand_thread.cc
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: wakeup_fd, server_thread, wakeup_event, id, lock.
false
1

gearmand_thread_st::gearmand_thread_st(gearmand_st& gearmand_):
  is_thread_lock(false),
------------
  • 20.
Viva64-EM
full
642
/nfs/home/dmka/src/gearmand/libgearman-server/io.cc
error
V555
The expression of the 'A - B > 0' kind will work as 'A != B'.
false
2
    /* If there is any room in the buffer, copy in data. */
    if (packet->data and (GEARMAND_SEND_BUFFER_SIZE - connection->send_buffer_size) > 0)
    {
------------
  • 21.
Viva64-EM
full
130
/nfs/home/dmka/src/gearmand/libgearman-server/log.cc
error
V519
The 'error_to_report' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 126, 130.
false
2
  case EHOSTDOWN:
    error_to_report= GEARMAND_LOST_CONNECTION;

126,130
------------
  • 22.
Viva64-EM
full
294
/nfs/home/dmka/src/gearmand/libgearman-server/log.cc
error
V618
It's dangerous to call the 'snprintf' function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf("%s", str);
false
2

    size_t ask= snprintf(0, 0, format);
    ask++; // for null
------------
  • 23.
Viva64-EM
full
437
/nfs/home/dmka/src/gearmand/libgearman-server/log.cc
error
V618
It's dangerous to call the 'snprintf' function in such a manner, as the line being passed could contain format specification. The example of the safe code: printf("%s", str);
false
2

      size_t ask= snprintf(0, 0, format);
      ask++; // for null
------------
  • 24.
Viva64-EM
full
812
/nfs/home/dmka/src/gearmand/libgearman-server/server.cc
error
V595
The 'server_job' pointer was utilized before it was verified against nullptr. Check lines: 812, 813.
false
1
                         "Exception being sent from: %.*s(%lu)",
                         server_job->function->function_name_size, server_job->function->function_name, server_job->function->function_name_size);
      if (server_job == NULL)
812,813
------------
  • 25.
Viva64-EM
full
386
/nfs/home/dmka/src/gearmand/libgearman-server/thread.cc
error
V575
The null pointer is passed into 'pthread_attr_setscope' function. Inspect the second argument.
false
1

  if ((error= pthread_attr_setscope(&attr, PTHREAD_SCOPE_SYSTEM)))
  {
-----------
  • 26.
Viva64-EM
full
66
/nfs/home/dmka/src/gearmand/libgearman-server/timer.cc
error
V512
A call of the 'memset' function will lead to underflow of the buffer 'fds'.
false
1
  {
    memset(fds, 0, sizeof(pollfd));
    fds[0].fd= -1; //STDIN_FILENO;
------------
  • 27.
Viva64-EM
full
83
/nfs/home/dmka/src/gearmand/libgearman-server/timer.cc
error
V575
The null pointer is passed into 'pthread_exit' function. Inspect the first argument.
false
1
      {
        pthread_exit(NULL);
      }
------------
  • 28.
Viva64-EM
full
89
/nfs/home/dmka/src/gearmand/libgearman-server/timer.cc
error
V575
The null pointer is passed into 'pthread_exit' function. Inspect the first argument.
false
1

  pthread_exit(NULL);
}
  • 29.
Viva64-EM
full
79
/nfs/home/dmka/src/gearmand/libgearman/check.cc
error
V526
The 'memcmp' function returns 0 if corresponding buffers are equal. Consider examining the condition for mistakes.
false
3

  if (memcmp(_workload, con->_packet.value(), compared))
  {
------------
  • 30.
Viva64-EM
full
79
/nfs/home/dmka/src/gearmand/libgearman/check.cc
error
V526
The 'memcmp' function returns 0 if corresponding buffers are equal. Consider examining the condition for mistakes.
false
3

  if (memcmp(_workload, con->_packet.value(), compared))
  {
------------
  • 31.
Viva64-EM
full
231
/nfs/home/dmka/src/gearmand/libgearman/add.cc
error
V595
The 'task->client' pointer was utilized before it was verified against nullptr. Check lines: 231, 243.
false
1
      {
        gearman_log_debug(task->client->universal, "uuid_generate_time_safe() failed or does not exist on this platform");
      }
231,243
------------
  • 32.
Viva64-EM
full
949
/nfs/home/dmka/src/gearmand/libgearman/client.cc
error
V528
It is odd that pointer to 'bool' type is compared with the false value. Probably meant: *is_known == false.
false
1

    if (is_known == false and is_running == false)
    {
------------
  • 33.
Viva64-EM
full
1618
/nfs/home/dmka/src/gearmand/libgearman/client.cc
error
V526
The 'strncmp' function returns 0 if corresponding strings are equal. Consider examining the condition for mistakes.
false
3
              { }
              else if (strncmp(client->task->impl()->job_handle,
                               static_cast<char *>(client->con->_packet.arg[0]),
------------
  • 34.
Viva64-EM
full
526
/nfs/home/dmka/src/gearmand/libgearman/connection.cc
error
V555
The expression '(8192 - send_buffer_size) > 0' will work as '8192 != send_buffer_size'.
false
2
    /* If there is any room in the buffer, copy in data. */
    if (packet_arg.data and (GEARMAN_SEND_BUFFER_SIZE - send_buffer_size) > 0)
    {
------------
  • 35.
Viva64-EM
full
91
/nfs/home/dmka/src/gearmand/libgearman/connection.cc
error
V730
It is possible that not all members of a class are initialized inside the constructor. Consider inspecting: send_buffer, recv_buffer.
false
3

gearman_connection_st::gearman_connection_st(gearman_universal_st &universal_arg, const char* host_, const char* service_):
  state(GEARMAN_CON_UNIVERSAL_ADDRINFO),
------------
  • 36.
Viva64-EM
full
671
/nfs/home/dmka/src/gearmand/libgearman/worker.cc
error
V506
Pointer to local variable 'unused' is stored outside the scope of this variable. Such a pointer will become invalid.
false
2
    {
      ret_ptr= &unused;
    }
------------
  • 37.
Viva64-EM
full
820
/nfs/home/dmka/src/gearmand/libgearman/worker.cc
error
V595
The 'worker' pointer was utilized before it was verified against nullptr. Check lines: 820, 826.
false
1
              }
              assert(worker->job()->impl());
            }
820,826
  • 38.
Viva64-EM
full
1339
/nfs/home/dmka/src/gearmand/libgearman/worker.cc
error
V507
Pointer to local array 'timeout_buffer' is stored outside the scope of this array. Such a pointer will become invalid.
false
2
    args_size[0]= function->length() + 1;
    args[1]= timeout_buffer;
    args_size[1]= strlen(timeout_buffer);
------------
  • 39.
Viva64-EM
full
58
/nfs/home/dmka/src/gearmand/libtest/error.h
error
V690
The 'Error' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class.
false
2

class Error
{
------------
  • 40.
Viva64-EM
full
41
/nfs/home/dmka/src/gearmand/libtest/exception.hpp
error
V690
The 'exception' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class.
false
2

class exception : public std::exception
{
------------
  • 41.
Viva64-EM
full
43
/nfs/home/dmka/src/gearmand/libtest/exception/disconnected.hpp
error
V690
The 'disconnected' class implements a copy constructor, but lacks the '=' operator. It is dangerous to use such a class.
false
2

class disconnected : public libtest::exception
{
------------
  • 42.
Viva64-EM
full
107
/nfs/home/dmka/src/gearmand/libtest/comparison.hpp
error
V526
The 'strncmp' function returns 0 if corresponding strings are equal. Consider examining the condition for mistakes.
false
3

  if (strncmp(__expected, __actual, strlen(__expected)))
  {
------------
  • 43.
Viva64-EM
full
59
/nfs/home/dmka/src/gearmand/libtest/thread.hpp
error
V509
The 'throw' operator inside the destructor should be placed within the try..catch block. Raising exception inside the destructor is illegal.
false
2
    {
      throw libtest::fatal(LIBYATL_DEFAULT_PARAM, "pthread_cond_destroy: %s", strerror(_err));
    }
------------
  • 44.
Viva64-EM
full
92
/nfs/home/dmka/src/gearmand/libtest/thread.hpp
error
V509
The 'throw' operator inside the destructor should be placed within the try..catch block. Raising exception inside the destructor is illegal.
false
2
    {
      throw libtest::fatal(LIBYATL_DEFAULT_PARAM, "pthread_mutex_unlock: %s", strerror(err));
    }
------------
  • 45.
Viva64-EM
full
132
/nfs/home/dmka/src/gearmand/libtest/thread.hpp
error
V509
The 'throw' operator inside the destructor should be placed within the try..catch block. Raising exception inside the destructor is illegal.
false
2
    {
      throw libtest::fatal(LIBYATL_DEFAULT_PARAM, "pthread_cond_destroy: %s", strerror(err));
    }
------------
  • 46.
Viva64-EM
full
55
/nfs/home/dmka/src/gearmand/libtest/client.cc
error
V730
It is possible that not all members of a class are initialized inside the constructor. Consider inspecting: _ai.
false
2

SimpleClient::SimpleClient(const std::string& hostname_, in_port_t port_) :
  _is_connected(false),
------------
  • 47.
Viva64-EM
full
76
/nfs/home/dmka/src/gearmand/libtest/has.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1
  char *getenv_ptr;
  if (bool((getenv_ptr= getenv("POSTGES_IS_RUNNING_AND_SETUP"))))
  {
------------
  • 48.
Viva64-EM
full
97
/nfs/home/dmka/src/gearmand/libtest/has.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1
    char *getenv_ptr;
    if (bool((getenv_ptr= getenv("PWD"))) and 
        ((strcmp(GEARMAND_BINARY, "./gearmand/gearmand") == 0) or (strcmp(GEARMAND_BINARY, "gearmand/gearmand") == 0)))
------------
  • 49.
Viva64-EM
full
98
/nfs/home/dmka/src/gearmand/libtest/has.cc
error
V549
The first argument of 'strcmp' function is equal to the second argument.
false
1
    if (bool((getenv_ptr= getenv("PWD"))) and 
        ((strcmp(GEARMAND_BINARY, "./gearmand/gearmand") == 0) or (strcmp(GEARMAND_BINARY, "gearmand/gearmand") == 0)))
    {
------------
  • 50.
Viva64-EM
full
71
/nfs/home/dmka/src/gearmand/libtest/is_local.cc
error
V601
The 'false' value is implicitly cast to the integer type. Inspect the third argument.
false
3
  {
    setenv("GEARMAND_CA_CERTIFICATE", YATL_CA_CERT_PEM, false);
    setenv("GEARMAND_SERVER_PEM", YATL_CERT_PEM, false);
------------
  • 51.
Viva64-EM
full
72
/nfs/home/dmka/src/gearmand/libtest/is_local.cc
error
V601
The 'false' value is implicitly cast to the integer type. Inspect the third argument.
false
3
    setenv("GEARMAND_CA_CERTIFICATE", YATL_CA_CERT_PEM, false);
    setenv("GEARMAND_SERVER_PEM", YATL_CERT_PEM, false);
    setenv("GEARMAND_SERVER_KEY", YATL_CERT_KEY_PEM, false);
------------
  • 52.
Viva64-EM
full
73
/nfs/home/dmka/src/gearmand/libtest/is_local.cc
error
V601
The 'false' value is implicitly cast to the integer type. Inspect the third argument.
false
3
    setenv("GEARMAND_SERVER_PEM", YATL_CERT_PEM, false);
    setenv("GEARMAND_SERVER_KEY", YATL_CERT_KEY_PEM, false);
    setenv("GEARMAND_CLIENT_PEM", YATL_CERT_PEM, false);
------------
  • 53.
Viva64-EM
full
74
/nfs/home/dmka/src/gearmand/libtest/is_local.cc
error
V601
The 'false' value is implicitly cast to the integer type. Inspect the third argument.
false
3
    setenv("GEARMAND_SERVER_KEY", YATL_CERT_KEY_PEM, false);
    setenv("GEARMAND_CLIENT_PEM", YATL_CERT_PEM, false);
    setenv("GEARMAND_CLIENT_KEY", YATL_CERT_KEY_PEM, false);
------------
  • 54.
Viva64-EM
full
75
/nfs/home/dmka/src/gearmand/libtest/is_local.cc
error
V601
The 'false' value is implicitly cast to the integer type. Inspect the third argument.
false
3
    setenv("GEARMAND_CLIENT_PEM", YATL_CERT_PEM, false);
    setenv("GEARMAND_CLIENT_KEY", YATL_CERT_KEY_PEM, false);
  }
------------
  • 55.
Viva64-EM
full
223
/nfs/home/dmka/src/gearmand/libtest/main.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1
  errno= 0;
  if (bool(getenv("YATL_REPEAT")))
  {
------------
  • 56.
Viva64-EM
full
234
/nfs/home/dmka/src/gearmand/libtest/main.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1

  if ((bool(getenv("YATL_QUIET")) and (strcmp(getenv("YATL_QUIET"), "0") == 0)) or opt_quiet)
  {
------------
  • 57.
Viva64-EM
full
240
/nfs/home/dmka/src/gearmand/libtest/main.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1
  {
    if (bool(getenv("YATL_QUIET")) and (strcmp(getenv("YATL_QUIET"), "1") == 0))
    { }
------------
  • 58.
Viva64-EM
full
248
/nfs/home/dmka/src/gearmand/libtest/main.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1

  if ((bool(getenv("YATL_WILDCARD"))))
  {
------------
  • 59.
Viva64-EM
full
253
/nfs/home/dmka/src/gearmand/libtest/main.cc
error
V001
A code fragment from '/nfs/home/dmka/src/gearmand/libtest/main.cc' cannot be analyzed.
false
3
------------
  • 60.
Viva64-EM
full
163
/nfs/home/dmka/src/gearmand/libtest/port.cc
error
V519
The 'sin.sin_addr.s_addr' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 162, 163.
false
2
        sin.sin_addr.s_addr= 0;
        sin.sin_addr.s_addr= INADDR_ANY;
        sin.sin_family= AF_INET;
162,163
------------
  • 61.
Viva64-EM
full
112
/nfs/home/dmka/src/gearmand/libtest/server.cc
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: _error_file, _error_line.
false
1

Server::Server(const std::string& host_arg, const in_port_t port_arg,
               const std::string& executable, const bool _is_libtool,
------------
  • 62.
Viva64-EM
full
93
/nfs/home/dmka/src/gearmand/libtest/signal.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1

  if (bool(getenv("LIBTEST_IN_GDB")) == false)
  {
------------
  • 63.
Viva64-EM
full
201
/nfs/home/dmka/src/gearmand/libtest/signal.cc
error
V542
Consider inspecting an odd type cast: 'char *' to 'bool'.
false
1
  sigemptyset(&set);
  if (bool(getenv("LIBTEST_IN_GDB")) == false)
  {
------------
  • 64.
Viva64-EM
full
195
/nfs/home/dmka/src/gearmand/libtest/signal.cc
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: __shutdown.
false
1

SignalThread::SignalThread() :
  magic_memory(MAGIC_MEMORY),
------------
  • 65.
Viva64-EM
full
54
/nfs/home/dmka/src/gearmand/libhostile/t/pipe.c
error
V575
The null pointer is passed into 'pipe' function. Inspect the first argument.
false
1
  {
    ASSERT_TRUE(pipe(NULL) == -1);
    ASSERT_TRUE(errno == EFAULT);
------------

This was referenced May 26, 2017
@p-alik
Copy link
Collaborator Author

p-alik commented Jun 1, 2017

38 task are done in branch.
In my opinion remaining are merely or unimportant.

@SpamapS
Copy link
Member

SpamapS commented Jun 1, 2017

This has been an epic task, thanks for taking it on @p-alik ! I will try and get to all the reviews soon.

illuusio pushed a commit to illuusio/gearmand that referenced this issue Nov 2, 2018
p-alik added a commit that referenced this issue Nov 27, 2018
@p-alik
Copy link
Collaborator Author

p-alik commented Dec 16, 2018

done in #214, #215 and #223

@p-alik p-alik closed this as completed Dec 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants