A few months ago, I check the Samba project by use PVS-Studio tool. But I forgot write about issues. Now remembered and decided to send. Maybe some bugs still present in the code.

-------------------------------------------------------------------------------
samba

V501 There are identical sub-expressions 'ace->type == SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT' to the left and to the right of the '||' operator. sddl.c 528

static char *sddl_encode_ace(TALLOC_CTX *mem_ctx, const struct security_ace *ace,
           const struct dom_sid *domain_sid)
{
  ....
  if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED_OBJECT ||
      ace->type == SEC_ACE_TYPE_ACCESS_DENIED_OBJECT ||
      ace->type == SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT ||
      ace->type == SEC_ACE_TYPE_SYSTEM_AUDIT_OBJECT) {
  ....
}
-------------------------------------------------------------------------------
samba

V501 There are identical sub-expressions to the left and to the right of the '!=' operator: s1_len != s1_len pdbtest.c 106

static bool samu_correct(struct samu *s1, struct samu *s2)
{
  ....
  } else if (s1_len != s1_len) {
    DEBUG(0, ("Password history not written correctly, lengths differ, want %d, got %d\n",
          s1_len, s2_len));
    ret = False;
  }
  ....
}
-------------------------------------------------------------------------------
samba

V501 There are identical sub-expressions to the left and to the right of the '>' operator: data > data t_asn1.c 45

int main(void)
{
  ....
    if ((data->length != tests[i].length) ||
        (memcmp(data>data, tests[i].data, data->length) != 0)) {
      printf("Test for %d failed\n", values[i]);
      ok = False;
    }
  ....
}

#info
Most likely this is what should be written here: data->data.
-------------------------------------------------------------------------------
samba

V501 There are identical sub-expressions to the left and to the right of the '>' operator: i2->pid > i2->pid brlock.c 1901

static int compare_procids(const void *p1, const void *p2)
{
  const struct server_id *i1 = (struct server_id *)p1;
  const struct server_id *i2 = (struct server_id *)p2;

  if (i1->pid < i2->pid) return -1;
  if (i2->pid > i2->pid) return 1;
  return 0;
}
-------------------------------------------------------------------------------
samba

V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((domain))' expression. wbinfo.c 124
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. wbinfo.c 125

#define FSTRING_LEN 256
typedef char fstring[FSTRING_LEN];

char *safe_strcpy(char *dest,const char *src, size_t maxlength)
#define fstrcpy(d,s) safe_strcpy((d),(s),sizeof(fstring)-1)

static bool parse_wbinfo_domain_user(
  const char *domuser, fstring domain, fstring user)
{
  ....
    /* Maybe it was a UPN? */
    if ((p = strchr(domuser, '@')) != NULL) {
      fstrcpy(domain, "");
      fstrcpy(user, domuser);
      return true;
    }
  ....
}

#add
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. wbinfo.c 129
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((domain))' expression. wbinfo.c 130
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. wbinfo.c 134
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((domain))' expression. wbinfo.c 135
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((fqdn))' expression. util.c 2215
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((fqdn))' expression. util.c 2243
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((srv_name))' expression. namequery_dc.c 137
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((unname))' expression. nmbd_workgroupdb.c 55
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((unname))' expression. nmbd_workgroupdb.c 59
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((sharename))' expression. printing.c 133
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof (new_name)' expression. mangle_hash2.c 705
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. password.c 647
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. password.c 685
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. password.c 698
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. password.c 717
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((name))' expression. net_rpc_rights.c 54
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((dcname))' expression. winbindd_cm.c 702
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. winbindd_util.c 864
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((domain))' expression. winbindd_util.c 867
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((domain))' expression. winbindd_util.c 869
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((user))' expression. winbindd_util.c 875
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((domain))' expression. winbindd_util.c 876
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((key))' expression. mapfile.c 90
V511 The sizeof() operator returns size of the pointer, and not of the array, in 'sizeof ((value))' expression. mapfile.c 91
-------------------------------------------------------------------------------
samba

V512 A call of the 'memset' function will lead to underflow of the buffer 'rt'. perf_writer.c 80

void initialize(PERF_DATA_BLOCK *data,
  RuntimeSettings *rt, int argc, char **argv)
{
    memset(data, 0, sizeof(*data));
    memset(rt, 0, sizeof(*data));
  ....
}

#info
Most likely this is what should be written here: memset(rt, 0, sizeof(*rt));.
-------------------------------------------------------------------------------
samba

V512 A call of the 'memcmp' function will lead to underflow of the buffer 'u0'. netuser.c 247

static NET_API_STATUS test_netusermodals(struct libnetapi_ctx *ctx,
           const char *hostname)
{
  ....
  struct USER_MODALS_INFO_0 *u0 = NULL;
  struct USER_MODALS_INFO_0 *_u0 = NULL;
  ....
  if (memcmp(u0, _u0, sizeof(u0) != 0)) {
    printf("USER_MODALS_INFO_0 struct has changed!!!!\n");
    return -1;
  }
  ....
}

#info
Most likely this is what should be written here: sizeof(*u0).
-------------------------------------------------------------------------------
samba

V523 The 'then' statement is equivalent to the 'else' statement. reg_perfcount.c 1092

static uint32 reg_perfcount_get_perf_data_block(....)
{
  ....
  if(object_ids == NULL)
  {
    /* we're getting a request for "Global" here */
    retval = _reg_perfcount_assemble_global(block, mem_ctx, base_index, names);
  }
  else
  {
    /* we're getting a request for a specific set of PERF_OBJECT_TYPES */
    retval = _reg_perfcount_assemble_global(block, mem_ctx, base_index, names);
  }
  ....
}
-------------------------------------------------------------------------------
samba

V547 Expression '(to != CH_UTF16LE) || (to != CH_UTF16BE)' is always true. Probably the '&&' operator should be used here. charcnv.c 188

static size_t convert_string_internal(....)
{
  ....
    if (((from == CH_UTF16LE)||(from == CH_UTF16BE)) &&
        ((to != CH_UTF16LE)||(to != CH_UTF16BE))) {
  ....
}

#add
V547 Expression '(to != CH_UTF16LE) || (to != CH_UTF16BE)' is always true. Probably the '&&' operator should be used here. charcnv.c 599
-------------------------------------------------------------------------------
samba

V547 Expression 'result.pid < 0' is always false. Unsigned type value is never < 0. util.c 2376

struct server_id {
  uint32_t pid;
  uint32_t vnn;
  uint64_t unique_id;
}

struct server_id interpret_pid(const char *pid_string)
{
  struct server_id result;
  ....
  /* Assigning to result.pid may have overflowed
     Map negative pid to -1: i.e. error */
  if (result.pid < 0) {
    result.pid = -1;
  }
  ....
}
-------------------------------------------------------------------------------
samba

V547 Expression is always false. Consider reviewing this expression. pdbtest.c 170

static bool samu_correct(struct samu *s1, struct samu *s2)
{
  ....
  if (d2_buf == NULL && d2_buf != NULL) {
    DEBUG(0, ("Logon hours is not set\n"));
    ret = False;
  }
  ....
}
-------------------------------------------------------------------------------
samba

V597 The compiler could delete the 'memset' function call, which is used to flush 'x' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. md2.c 91

static void
calc(struct md2 *m, const void *v)
{
  unsigned char x[48], L;
  ....
  memset(x, 0, sizeof(x));
}

#add
V597 The compiler could delete the 'memset' function call, which is used to flush 'buf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. rand-fortuna.c 291
V597 The compiler could delete the 'memset' function call, which is used to flush 'hash' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. rand-fortuna.c 341
V597 The compiler could delete the 'memset' function call, which is used to flush 'buf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. rand-fortuna.c 378
V597 The compiler could delete the 'memset' function call, which is used to flush 'k' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. rc2.c 110
-------------------------------------------------------------------------------
samba

V610 Undefined behavior. Check the shift operator '<<. The left operand '~0' is negative. idtree.c 284

static void *_idr_find(struct idr_context *idp, int id)
{
  ....
  if (n + IDR_BITS < 31 &&
      ((id & ~(~0 << MAX_ID_SHIFT)) >> (n + IDR_BITS))) {
    return NULL;
  }
  ....
}
-------------------------------------------------------------------------------
samba

V627 Consider inspecting the expression. The argument of sizeof() is the macro which expands to a number. regfio.c 1470

#define HBIN_HDR_SIZE    4

char header[HBIN_HDR_SIZE];

static REGF_HBIN* regf_hbin_allocate( REGF_FILE *file, uint32 block_size )
{
  REGF_HBIN *hbin;
  ....
  memcpy( hbin->header, "hbin", sizeof(HBIN_HDR_SIZE) );
  ....
}

#info
It works. Good luck: sizeof(4) == 4.
-------------------------------------------------------------------------------
samba

V595 The 'ctx' pointer was utilized before it was verified against nullptr. Check lines: 67, 72. cm.c 67

static WERROR libnetapi_open_ipc_connection(struct libnetapi_ctx *ctx,
              const char *server_name,
              struct client_ipc_connection **pp)
{
  struct libnetapi_private_ctx *priv_ctx =
    (struct libnetapi_private_ctx *)ctx->private_data;
  struct user_auth_info *auth_info = NULL;
  struct cli_state *cli_ipc = NULL;
  struct client_ipc_connection *p;

  if (!ctx || !pp || !server_name) {
    return WERR_INVALID_PARAM;
  }
  ....
}
-------------------------------------------------------------------------------
samba

V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. reg_perfcount.c 945

static bool _reg_perfcount_init_data_block(....)
{
  smb_ucs2_t *temp = NULL;
  ....
  memset(temp, 0, sizeof(temp));
  ....
}

#info
Most likely this is what should be written here: sizeof(*temp).

#add
V579 The cli_api function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the seventh argument. clirap2.c 331
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. engine.c 91
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. md2.c 133