I've tested the our PVS-Studio analyzer with the projects Apache. While analyzing this project I've found some mistakes and decided to mention it. Hope it will come in handy. See also this article: http://www.viva64.com/en/b/0105/ Bugs: V501 There are identical sub-expressions 'state == rsl_encoding' to the left and to the right of the '||' operator. mod_mime_magic mod_mime_magic.c 787 static int magic_rsl_to_request(request_rec *r) { ... if (state == rsl_subtype || state == rsl_encoding || state == rsl_encoding) { ... } Need: if (state == rsl_subtype || state == rsl_separator || state == rsl_encoding) { --------------------------------------------------------------- V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 560 #define MEMSET_BZERO(p,l) memset((p), 0, (l)) void apr__SHA256_Final(sha2_byte digest[], SHA256_CTX* context) { ... MEMSET_BZERO(context, sizeof(context)); ... } And here: False 1 902 V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 581 False False 1 916 V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 892 False False 1 917 V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 912 False False 1 918 V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 967 False False 1 919 V512 A call of the 'memset' function will lead to underflow of the buffer '(context)'. apr sha2.c 987 False --------------------------------------------------------------- V527 It is odd that the '\0' value is assigned to 'char' type pointer. Probably meant: *tag->arg = '\0'. mod_headers mod_headers.c 330 typedef struct { const char* (*func)(request_rec *r,char *arg); char *arg; } format_tag; static char *parse_format_tag(apr_pool_t *p, format_tag *tag, const char **sa) { ... tag->arg = '\0'; ... } Need: tag->arg = NULL; Or: tag->arg[0] = '\0'; --------------------------------------------------------------- V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: '(*pointer)++'. apriconv iconv_uc.c 114 apr_status_t iconv_uc_conv(..., apr_size_t *res) { ... if (size) *res ++; ... } Need: (*res)++; --------------------------------------------------------------- V547 Expression 'len < 0' is always false. Unsigned type value is never < 0. aprutil apr_memcache.c 814 typedef size_t apr_size_t; APU_DECLARE(apr_status_t) apr_memcache_getp(...) { ... apr_size_t len = 0; ... len = atoi(length); ... if (len < 0) { *new_length = 0; *baton = NULL; } else { ... } } --------------------------------------------------------------- V547 Expression 'csd < 0' is always false. Unsigned type value is never < 0. libhttpd child.c 404 typedef UINT_PTR SOCKET; static unsigned int __stdcall win9x_accept(void * dummy) { SOCKET csd; ... do { clen = sizeof(sa_client); csd = accept(nsd, (struct sockaddr *) &sa_client, &clen); } while (csd < 0 && APR_STATUS_IS_EINTR(apr_get_netos_error())); ... } --------------------------------------------------------------- V560 A part of conditional expression is always true: 0x04. pcre pcre.c 3366 static BOOL compile_branch(...) { ... else if ((digitab[ptr[1]] && ctype_digit) != 0) ... } Need: else if ((digitab[ptr[1]] & ctype_digit) != 0) --------------------------------------------------------------- V568 It's odd that the argument of sizeof() operator is the 'sizeof (SECURITY_ATTRIBUTES)' expression. libhttpd util_win32.c 115 PSECURITY_ATTRIBUTES GetNullACL(void) { PSECURITY_ATTRIBUTES sa; sa = (PSECURITY_ATTRIBUTES) LocalAlloc(LPTR, sizeof(SECURITY_ATTRIBUTES)); sa->nLength = sizeof(sizeof(SECURITY_ATTRIBUTES)); ... } Need: sa->nLength = sizeof(SECURITY_ATTRIBUTES); --------------------------------------------------------------- V579 The apr_snprintf function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the second argument. libhttpd util_pcre.c 85 AP_DECLARE(apr_size_t) ap_regerror(int errcode, const ap_regex_t *preg, char *errbuf, apr_size_t errbuf_size) { ... apr_snprintf(errbuf, sizeof errbuf, "%s%s%-6d", message, addmessage, (int)preg->re_erroffset);
Very cool/thanks for the report! (child.c 404 is already fixed, but the others aren't familiar)
- util_pcre.c 85 already fixed in r1095448 - other HTTPD issues fixed in trunk in r1172732 - the APR parts still need to be checked
- fixed apr sha2.c issue in 1172825, r1172828, r1172829 - apr memcache issue already fixed in trunk in r982408, r982409 - Checked that the pcre issue is fixed in 8.12 Can somebody look at iconv_uc.c?
Looks that all is solved now, except the iconv potential issue. Re-affecting to APR for it. > V532 Consider inspecting the statement of '*pointer++' pattern. Probably meant: > '(*pointer)++'. apriconv iconv_uc.c 114 > > apr_status_t > iconv_uc_conv(..., apr_size_t *res) > { > ... > if (size) > *res ++; > ... > } > > Need: (*res)++;