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

Fix some compilation warnings. #33

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/config/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <assert.h>
#include <config/config.h>
#include <util/log.h>
#include <util/util.h>
Expand Down Expand Up @@ -106,12 +107,16 @@ static const char* prev_item(const char* opt, const char* end)
{
beg--;
}
int len = (int)(end - beg);
assert(beg <= end);
size_t len = (size_t)(end - beg);
Comment on lines +110 to +111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, from the five lines before it should already be clear that beg <= end.
So I do not see the need for the assertion and thus also not for the include <assert.h>.
Yet maybe append to the next (assignment) line a comment such as: /* note that beg <= end */.

if(len > SECTION_NAME_MAX)
{
len = SECTION_NAME_MAX;
}
strncpy(opt_item, beg, len);
if(len > 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard is not needed since it is equivalent to len != 0, and if len == 0, strncpy() will do nothing anyway.

{
strncpy(opt_item, beg, len);
}
opt_item[len] = '\0';
if(end - beg > SECTION_NAME_MAX)
{
Expand Down
42 changes: 28 additions & 14 deletions src/config/config_update.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <assert.h>
#include <config/config_update.h>
#include <storage/files_icv.h>
#include <util/log.h>
Expand All @@ -29,9 +30,9 @@ static void skip_space(char** p)
}


static bool copy_line_substring(char* dest, const char* src, int* offset, int max_dest_len)
static bool copy_line_substring(char* dest, const char* src, size_t* offset, size_t max_dest_len)
{
int copy_len = strnlen(src, max_dest_len);
size_t copy_len = strnlen(src, max_dest_len);
if(*offset + copy_len > max_dest_len)
{
return false;
Expand All @@ -51,9 +52,9 @@ static int file_modified;
* Keeps any end-of-line comment.
* Returns the length of the resulting line, or 0 in case of error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct the comment: or -1 in case of error.

*/
static int refactor_entry(char* src_p, char* dest_p, const char* const key_p, const char* const val_p, int max_dest_len)
static int refactor_entry(char* const src_p, char* dest_p, const char* const key_p, const char* const val_p, size_t max_dest_len)
{
int line_len = 0;
size_t line_len = 0;
char* pos_p = src_p;

if(0 is_eq src_p or 0 is_eq key_p or 0 is_eq val_p)
Expand Down Expand Up @@ -82,8 +83,9 @@ static int refactor_entry(char* src_p, char* dest_p, const char* const key_p, co
skip_space(&pos_p);

/* found "key = ", copy in the new value */
int val_len = strnlen(val_p, max_dest_len);
line_len = pos_p - src_p;
const size_t val_len = strnlen(val_p, max_dest_len);
assert(pos_p >= src_p);
line_len = (size_t)(pos_p - src_p);
Comment on lines +87 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here, I suggest replacing the new assertion by, e.g., /* note that pos_p >= src_p */ at the end of the following line
and removing the #include <assert.h>.

if(not copy_line_substring(dest_p, val_p, &line_len, max_dest_len))
{
goto len_err;
Expand Down Expand Up @@ -151,11 +153,11 @@ static size_t read_config_until_section(FILE* file_p, const char* file_name, con
{
size_t file_len = 0;
char* pos_p = 0;
int section_name_len = strlen(section_name);
const size_t section_name_len = strlen(section_name);
LOG(FL_TRACE, "Reading configuration until section is found or end of file reached");
while(0 not_eq fgets(input_line, c_line_buf_size, file_p))
{
int line_len = strnlen(input_line, c_line_buf_size);
size_t line_len = strnlen(input_line, c_line_buf_size);
copy_file_line(file_buffer, input_line, line_len);

/* break if line matches "\ *\[\ *section_name\ *\]" */
Expand Down Expand Up @@ -199,7 +201,13 @@ static size_t add_to_config_section(size_t file_len, const key_val_section* cons
}
if(pair->key and not found[i])
{
int line_len = snprintf(updated_line, c_line_buf_size, "%s = %s\n", pair->key, pair->val);
int line_len_aux = snprintf(updated_line, c_line_buf_size, "%s = %s\n", pair->key, pair->val);
if (line_len_aux < 0)
{
LOG(FL_ERR, "Failed to write string");
return 0;
}
size_t line_len = (size_t)line_len_aux;
copy_file_line(file_buffer, updated_line, line_len);
LOG(FL_TRACE, "Adding in config file: %s" /*\n*/, updated_line);
file_modified = 1;
Expand All @@ -221,7 +229,7 @@ static size_t update_config_section(FILE* file_p, const char* file_name, size_t
LOG(FL_TRACE, "Replacing 'key = value' pairs in the section");
while(0 not_eq fgets(input_line, c_line_buf_size, file_p))
{
int line_len = strnlen(input_line, c_line_buf_size);
size_t line_len = strnlen(input_line, c_line_buf_size);
if(line_len > c_line_buf_size - 1)
{
return 0;
Expand Down Expand Up @@ -261,7 +269,13 @@ static size_t update_config_section(FILE* file_p, const char* file_name, size_t
return 0;
}
found[i] = 1;
line_len = refactor_entry(input_line, updated_line, pair->key, pair->val, c_line_buf_size - 1);
int line_len_aux = refactor_entry(input_line, updated_line, pair->key, pair->val, c_line_buf_size - 1);
if (line_len_aux < 0)
{
// error already logged
return 0;
}
line_len = (size_t)line_len_aux;
copy_file_line(file_buffer, updated_line, line_len);
LOG(FL_TRACE, "Updating in config file: %s" /*\n*/, updated_line);
}
Expand All @@ -283,9 +297,9 @@ static size_t update_config_section(FILE* file_p, const char* file_name, size_t


/* copy the rest of the file into the buffer */
static int copy_remaining_config(FILE* file_p, int file_len, char* input_line)
static size_t copy_remaining_config(FILE* file_p, size_t file_len, char* input_line)
{
int line_len = strnlen(input_line, c_line_buf_size); /* first line of next section already read */
size_t line_len = strnlen(input_line, c_line_buf_size); /* first line of next section already read */
copy_file_line(file_buffer, input_line, line_len);

while(0 not_eq fgets(input_line, c_line_buf_size, file_p))
Expand Down Expand Up @@ -328,7 +342,7 @@ int CONF_update_config(OPTIONAL uta_ctx* ctx, const char* file_name, const key_v
return 0;
}

char* found = OPENSSL_malloc(key_val_section->count);
char* found = OPENSSL_malloc((size_t)key_val_section->count);
if(found is_eq 0)
{
LOG(FL_ERR, "Cannot update config, out of memory");
Expand Down
4 changes: 3 additions & 1 deletion src/connections/conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <assert.h>
#include <util/log.h>
#include <connections/conn.h>

Expand Down Expand Up @@ -120,7 +121,8 @@ char* CONN_get_host(const char* uri)
{
end = strchr(uri, '/');
}
int len = end not_eq 0 ? end - uri : strlen(uri);
assert(end is_eq 0 or end >= uri);
size_t len = end not_eq 0 ? (size_t)(end - uri) : strlen(uri);
Comment on lines +124 to +125
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here, I suggest not using assert() but optionally just adding a comment.

str = OPENSSL_strndup(uri, len);
if(0 is_eq str)
{
Expand Down
2 changes: 1 addition & 1 deletion src/connections/tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ SSL_CTX* TLS_CTX_new(OPTIONAL SSL_CTX* ssl_ctx,
bak_flags = X509_VERIFY_PARAM_get_flags(vpm);
/* disable any cert status/revocation checking etc. */
X509_VERIFY_PARAM_clear_flags(vpm,
compl(X509_V_FLAG_USE_CHECK_TIME
compl((unsigned long)X509_V_FLAG_USE_CHECK_TIME
bitor X509_V_FLAG_NO_CHECK_TIME));
X509_VERIFY_PARAM_set_flags(vpm, X509_V_FLAG_NONFINAL_CHECK);
}
Expand Down
6 changes: 3 additions & 3 deletions src/credentials/cert.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,17 +228,17 @@ void CERT_print(OPTIONAL const X509* cert, OPTIONAL BIO* bio, unsigned long neg_
unsigned long flags =
ASN1_STRFLGS_RFC2253 bitor ASN1_STRFLGS_ESC_QUOTE bitor XN_FLAG_SEP_CPLUS_SPC bitor XN_FLAG_FN_SN;
BIO_printf(bio, " Certificate\n");
X509_print_ex(bio, (X509*)cert, flags, compl X509_FLAG_NO_SUBJECT);
X509_print_ex(bio, (X509*)cert, flags, compl (unsigned long)X509_FLAG_NO_SUBJECT);
if(X509_check_issued((X509*)cert, (X509*)cert) is_eq X509_V_OK)
{
BIO_printf(bio, " self-signed\n");
}
else
{
BIO_printf(bio, " ");
X509_print_ex(bio, (X509*)cert, flags, compl X509_FLAG_NO_ISSUER);
X509_print_ex(bio, (X509*)cert, flags, compl (unsigned long)X509_FLAG_NO_ISSUER);
}
X509_print_ex(bio, (X509*)cert, flags, compl(X509_FLAG_NO_SERIAL bitor X509_FLAG_NO_VALIDITY));
X509_print_ex(bio, (X509*)cert, flags, compl((unsigned long)X509_FLAG_NO_SERIAL bitor X509_FLAG_NO_VALIDITY));
if(X509_cmp_current_time(X509_get0_notBefore(cert)) > 0)
{
BIO_printf(bio, " not yet valid\n");
Expand Down
28 changes: 14 additions & 14 deletions src/util/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

#include <assert.h>
#include <dirent.h>
#include <sys/stat.h>

Expand Down Expand Up @@ -42,7 +43,7 @@ int UTIL_atoint(const char* str)

const char* UTIL_skip_string(const char* s, OPTIONAL const char* p)
{
const int len_s = strlen(s);
const size_t len_s = strlen(s);
if(p not_eq 0 and 0 is_eq strncmp(p, s, len_s))
{
p += len_s;
Expand Down Expand Up @@ -102,7 +103,7 @@ void* UTIL_read_file(const char* filename, int* lenp)
FILE* fp = 0;
struct stat st;
unsigned char* contents = 0;
int contents_len = 0;
size_t contents_len = 0;

if(stat(filename, &st) < 0)
{
Expand All @@ -117,7 +118,7 @@ void* UTIL_read_file(const char* filename, int* lenp)
return 0;
}

contents_len = st.st_size;
contents_len = (size_t)st.st_size;
contents = OPENSSL_malloc(contents_len + 1);
if(contents is_eq 0)
{
Expand Down Expand Up @@ -525,18 +526,18 @@ bool UTIL_hex_to_bytes(const char** in_p, unsigned char* out, unsigned int num_o
num_out *= HEX_CHARS_PER_BYTE;
for(i = 0; i < num_out; i++)
{
char c = *((*in_p)++);
int c = *((*in_p)++);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?
Note that *in_p is of type const char*

if(('0' <= c) and (c <= '9'))
{
v = c - '0';
v = (unsigned int)(c - '0');
}
else if(('A' <= c) and (c <= 'F'))
{
v = (c - 'A') + (MAX_DIGIT + 1);
v = (unsigned int)(c - 'A') + (MAX_DIGIT + 1);
}
else if(('a' <= c) and (c <= 'f'))
{
v = (c - 'a') + (MAX_DIGIT + 1);
v = (unsigned int)(c - 'a') + (MAX_DIGIT + 1);
}
else
{
Expand Down Expand Up @@ -571,18 +572,16 @@ int UTIL_base64_encode_to_buf(const unsigned char* data, int len, char* buf, int
BIO_write(bio_mem, data, len);
(void)BIO_flush(bio_mem);
BIO_get_mem_ptr(bio_mem, &bptr);
int encoded_len = bptr->length;
size_t encoded_len = bptr->length;
if(encoded_len < buf_size)
{
memcpy(buf, bptr->data, encoded_len);
buf[encoded_len] = '\0';
}
else
{
encoded_len = -1 - 1;
BIO_free_all(bio_mem);
return encoded_len;
}
BIO_free_all(bio_mem);
return encoded_len;
return -2;
}


Expand Down Expand Up @@ -612,6 +611,7 @@ unsigned char* UTIL_base64_decode(const char* b64_data, int b64_len, int* decode
{
(*decoded_len)--;
}
assert(*decoded_len >= 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here, I suggest replacing the assertion by a comment.


/* Create a base64 filter */
BIO* bio_b64 = BIO_new(BIO_f_base64());
Expand All @@ -638,7 +638,7 @@ unsigned char* UTIL_base64_decode(const char* b64_data, int b64_len, int* decode
BIO_push(bio_b64, bio_mem);

/* Execute the base 64 decoding and store the output in the decoded_data memory++*/
unsigned char* decoded_data = OPENSSL_malloc(*decoded_len + 1);
unsigned char* decoded_data = OPENSSL_malloc((size_t)(*decoded_len) + 1);
if(decoded_data is_eq 0)
{
LOG_err("Failure allocate memory for base64 decoding");
Expand Down