From 77d0d190057d0be37215135c276d515ce82b06cd Mon Sep 17 00:00:00 2001 From: Martin Matuska Date: Tue, 14 Jan 2020 14:40:11 +0100 Subject: [PATCH] Multiple code fixes and optimizations archive_read_support_format_rar5.c: Bitfield int -> signed int Archive_write_set_format_iso9660.c: Bitfield int -> signed int archive_write_set_format_xar.c: Bitfield int -> signed int archive_write_set_format_7zip.c: Bitfield int -> signed int archive_read_support_format_xar.c Remove useless comparsion archive_read_support_format_rar.c: Fix invalid nested loop break. Comment out dead code sections. Simplify size comparsions of lensymbol and offsymbol. archive_read_support_filter_uu.c: Remove useless comparsions archive_read_disk_posix.c: Remove useless do-while-zero Found by LGTM.com code analysis --- libarchive/archive_entry.c | 10 ++-- libarchive/archive_read_disk_posix.c | 34 ++++++------- libarchive/archive_read_support_filter_uu.c | 30 +++++------ libarchive/archive_read_support_format_rar.c | 51 +++++++++++++------ libarchive/archive_read_support_format_rar5.c | 2 +- libarchive/archive_read_support_format_xar.c | 17 +++---- libarchive/archive_write_set_format_7zip.c | 2 +- libarchive/archive_write_set_format_iso9660.c | 14 ++--- libarchive/archive_write_set_format_xar.c | 6 +-- 9 files changed, 90 insertions(+), 76 deletions(-) diff --git a/libarchive/archive_entry.c b/libarchive/archive_entry.c index 72c644e60..a15e98c28 100644 --- a/libarchive/archive_entry.c +++ b/libarchive/archive_entry.c @@ -1699,7 +1699,7 @@ static const struct flag { const wchar_t *wname; unsigned long set; unsigned long clear; -} flags[] = { +} fileflags[] = { /* Preferred (shorter) names per flag first, all prefixed by "no" */ #ifdef SF_APPEND { "nosappnd", L"nosappnd", SF_APPEND, 0}, @@ -1876,7 +1876,7 @@ ae_fflagstostr(unsigned long bitset, unsigned long bitclear) bits = bitset | bitclear; length = 0; - for (flag = flags; flag->name != NULL; flag++) + for (flag = fileflags; flag->name != NULL; flag++) if (bits & (flag->set | flag->clear)) { length += strlen(flag->name) + 1; bits &= ~(flag->set | flag->clear); @@ -1889,7 +1889,7 @@ ae_fflagstostr(unsigned long bitset, unsigned long bitclear) return (NULL); dp = string; - for (flag = flags; flag->name != NULL; flag++) { + for (flag = fileflags; flag->name != NULL; flag++) { if (bitset & flag->set || bitclear & flag->clear) { sp = flag->name + 2; } else if (bitset & flag->clear || bitclear & flag->set) { @@ -1941,7 +1941,7 @@ ae_strtofflags(const char *s, unsigned long *setp, unsigned long *clrp) *end != ' ' && *end != ',') end++; length = end - start; - for (flag = flags; flag->name != NULL; flag++) { + for (flag = fileflags; flag->name != NULL; flag++) { size_t flag_length = strlen(flag->name); if (length == flag_length && memcmp(start, flag->name, length) == 0) { @@ -2009,7 +2009,7 @@ ae_wcstofflags(const wchar_t *s, unsigned long *setp, unsigned long *clrp) *end != L' ' && *end != L',') end++; length = end - start; - for (flag = flags; flag->wname != NULL; flag++) { + for (flag = fileflags; flag->wname != NULL; flag++) { size_t flag_length = wcslen(flag->wname); if (length == flag_length && wmemcmp(start, flag->wname, length) == 0) { diff --git a/libarchive/archive_read_disk_posix.c b/libarchive/archive_read_disk_posix.c index f7c96bffa..52fec7bb4 100644 --- a/libarchive/archive_read_disk_posix.c +++ b/libarchive/archive_read_disk_posix.c @@ -729,27 +729,23 @@ _archive_read_data_block(struct archive *_a, const void **buff, if ((t->flags & needsRestoreTimes) != 0 && t->restore_time.noatime == 0) flags |= O_NOATIME; - do { #endif - t->entry_fd = open_on_current_dir(t, - tree_current_access_path(t), flags); - __archive_ensure_cloexec_flag(t->entry_fd); + t->entry_fd = open_on_current_dir(t, + tree_current_access_path(t), flags); + __archive_ensure_cloexec_flag(t->entry_fd); #if defined(O_NOATIME) - /* - * When we did open the file with O_NOATIME flag, - * if successful, set 1 to t->restore_time.noatime - * not to restore an atime of the file later. - * if failed by EPERM, retry it without O_NOATIME flag. - */ - if (flags & O_NOATIME) { - if (t->entry_fd >= 0) - t->restore_time.noatime = 1; - else if (errno == EPERM) { - flags &= ~O_NOATIME; - continue; - } - } - } while (0); + /* + * When we did open the file with O_NOATIME flag, + * if successful, set 1 to t->restore_time.noatime + * not to restore an atime of the file later. + * if failed by EPERM, retry it without O_NOATIME flag. + */ + if (flags & O_NOATIME) { + if (t->entry_fd >= 0) + t->restore_time.noatime = 1; + else if (errno == EPERM) + flags &= ~O_NOATIME; + } #endif if (t->entry_fd < 0) { archive_set_error(&a->archive, errno, diff --git a/libarchive/archive_read_support_filter_uu.c b/libarchive/archive_read_support_filter_uu.c index 641297990..67ddffb06 100644 --- a/libarchive/archive_read_support_filter_uu.c +++ b/libarchive/archive_read_support_filter_uu.c @@ -574,14 +574,13 @@ read_more: while (l > 0) { int n = 0; - if (l > 0) { - if (!uuchar[b[0]] || !uuchar[b[1]]) - break; - n = UUDECODE(*b++) << 18; - n |= UUDECODE(*b++) << 12; - *out++ = n >> 16; total++; - --l; - } + if (!uuchar[b[0]] || !uuchar[b[1]]) + break; + n = UUDECODE(*b++) << 18; + n |= UUDECODE(*b++) << 12; + *out++ = n >> 16; total++; + --l; + if (l > 0) { if (!uuchar[b[0]]) break; @@ -626,14 +625,13 @@ read_more: while (l > 0) { int n = 0; - if (l > 0) { - if (!base64[b[0]] || !base64[b[1]]) - break; - n = base64num[*b++] << 18; - n |= base64num[*b++] << 12; - *out++ = n >> 16; total++; - l -= 2; - } + if (!base64[b[0]] || !base64[b[1]]) + break; + n = base64num[*b++] << 18; + n |= base64num[*b++] << 12; + *out++ = n >> 16; total++; + l -= 2; + if (l > 0) { if (*b == '=') break; diff --git a/libarchive/archive_read_support_format_rar.c b/libarchive/archive_read_support_format_rar.c index fe5903b64..98efbb1a6 100644 --- a/libarchive/archive_read_support_format_rar.c +++ b/libarchive/archive_read_support_format_rar.c @@ -148,6 +148,9 @@ #define FILE_ATTRIBUTE_DIRECTORY 0x10 #endif +#undef minimum +#define minimum(a, b) ((a)<(b)?(a):(b)) + /* Fields common to all headers */ struct rar_header { @@ -2470,8 +2473,11 @@ create_code(struct archive_read *a, struct huffman_code *code, if (add_value(a, code, j, codebits, i) != ARCHIVE_OK) return (ARCHIVE_FATAL); codebits++; - if (--symbolsleft <= 0) { break; break; } + if (--symbolsleft <= 0) + break; } + if (symbolsleft <= 0) + break; codebits <<= 1; } return (ARCHIVE_OK); @@ -2481,7 +2487,8 @@ static int add_value(struct archive_read *a, struct huffman_code *code, int value, int codebits, int length) { - int repeatpos, lastnode, bitpos, bit, repeatnode, nextnode; + int lastnode, bitpos, bit; + /* int repeatpos, repeatnode, nextnode; */ free(code->table); code->table = NULL; @@ -2491,6 +2498,9 @@ add_value(struct archive_read *a, struct huffman_code *code, int value, if(length < code->minlength) code->minlength = length; + /* + * Dead code, repeatpos was is -1 + * repeatpos = -1; if (repeatpos == 0 || (repeatpos >= 0 && (((codebits >> (repeatpos - 1)) & 3) == 0 @@ -2500,6 +2510,7 @@ add_value(struct archive_read *a, struct huffman_code *code, int value, "Invalid repeat position"); return (ARCHIVE_FATAL); } + */ lastnode = 0; for (bitpos = length - 1; bitpos >= 0; bitpos--) @@ -2515,9 +2526,12 @@ add_value(struct archive_read *a, struct huffman_code *code, int value, return (ARCHIVE_FATAL); } + /* + * Dead code, repeatpos was -1, bitpos >=0 + * if (bitpos == repeatpos) { - /* Open branch check */ + * Open branch check * if (!(code->tree[lastnode].branches[bit] < 0)) { archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, @@ -2536,16 +2550,17 @@ add_value(struct archive_read *a, struct huffman_code *code, int value, return (ARCHIVE_FATAL); } - /* Set branches */ + * Set branches * code->tree[lastnode].branches[bit] = repeatnode; code->tree[repeatnode].branches[bit] = repeatnode; code->tree[repeatnode].branches[bit^1] = nextnode; lastnode = nextnode; - bitpos++; /* terminating bit already handled, skip it */ + bitpos++; * terminating bit already handled, skip it * } else { + */ /* Open branch check */ if (code->tree[lastnode].branches[bit] < 0) { @@ -2559,7 +2574,7 @@ add_value(struct archive_read *a, struct huffman_code *code, int value, /* set to branch */ lastnode = code->tree[lastnode].branches[bit]; - } + /* } */ } if (!(code->tree[lastnode].branches[0] == -1 @@ -2643,11 +2658,15 @@ make_table_recurse(struct archive_read *a, struct huffman_code *code, int node, table[i].value = code->tree[node].branches[0]; } } + /* + * Dead code, node >= 0 + * else if (node < 0) { for(i = 0; i < currtablesize; i++) table[i].length = -1; } + */ else { if(depth == maxdepth) @@ -2679,6 +2698,10 @@ expand(struct archive_read *a, int64_t end) 0, 1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5 }; + static const int lengthb_min = minimum( + (int)(sizeof(lengthbases)/sizeof(lengthbases[0])), + (int)(sizeof(lengthbits)/sizeof(lengthbits[0])) + ); static const unsigned int offsetbases[] = { 0, 1, 2, 3, 4, 6, 8, 12, 16, 24, 32, 48, @@ -2696,6 +2719,10 @@ expand(struct archive_read *a, int64_t end) 11, 11, 12, 12, 13, 13, 14, 14, 15, 15, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 16, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18 }; + static const int offsetb_min = minimum( + (int)(sizeof(offsetbases)/sizeof(offsetbases[0])), + (int)(sizeof(offsetbits)/sizeof(offsetbits[0])) + ); static const unsigned char shortbases[] = { 0, 4, 8, 16, 32, 64, 128, 192 }; static const unsigned char shortbits[] = @@ -2775,9 +2802,7 @@ expand(struct archive_read *a, int64_t end) if ((lensymbol = read_next_symbol(a, &rar->lengthcode)) < 0) goto bad_data; - if (lensymbol > (int)(sizeof(lengthbases)/sizeof(lengthbases[0]))) - goto bad_data; - if (lensymbol > (int)(sizeof(lengthbits)/sizeof(lengthbits[0]))) + if (lensymbol > lengthb_min) goto bad_data; len = lengthbases[lensymbol] + 2; if (lengthbits[lensymbol] > 0) { @@ -2809,9 +2834,7 @@ expand(struct archive_read *a, int64_t end) } else { - if (symbol-271 > (int)(sizeof(lengthbases)/sizeof(lengthbases[0]))) - goto bad_data; - if (symbol-271 > (int)(sizeof(lengthbits)/sizeof(lengthbits[0]))) + if (symbol-271 > lengthb_min) goto bad_data; len = lengthbases[symbol-271]+3; if(lengthbits[symbol-271] > 0) { @@ -2823,9 +2846,7 @@ expand(struct archive_read *a, int64_t end) if ((offssymbol = read_next_symbol(a, &rar->offsetcode)) < 0) goto bad_data; - if (offssymbol > (int)(sizeof(offsetbases)/sizeof(offsetbases[0]))) - goto bad_data; - if (offssymbol > (int)(sizeof(offsetbits)/sizeof(offsetbits[0]))) + if (offssymbol > offsetb_min) goto bad_data; offs = offsetbases[offssymbol]+1; if(offsetbits[offssymbol] > 0) diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c index ce38b1fc9..6e91eab85 100644 --- a/libarchive/archive_read_support_format_rar5.c +++ b/libarchive/archive_read_support_format_rar5.c @@ -211,7 +211,7 @@ struct comp_state { or just a part of it. */ uint8_t block_parsing_finished : 1; - int notused : 4; + signed int notused : 4; int flags; /* Uncompression flags. */ int method; /* Uncompression algorithm method. */ diff --git a/libarchive/archive_read_support_format_xar.c b/libarchive/archive_read_support_format_xar.c index 34253a52f..7f8be398c 100644 --- a/libarchive/archive_read_support_format_xar.c +++ b/libarchive/archive_read_support_format_xar.c @@ -2613,15 +2613,14 @@ strappend_base64(struct xar *xar, while (l > 0) { int n = 0; - if (l > 0) { - if (base64[b[0]] < 0 || base64[b[1]] < 0) - break; - n = base64[*b++] << 18; - n |= base64[*b++] << 12; - *out++ = n >> 16; - len++; - l -= 2; - } + if (base64[b[0]] < 0 || base64[b[1]] < 0) + break; + n = base64[*b++] << 18; + n |= base64[*b++] << 12; + *out++ = n >> 16; + len++; + l -= 2; + if (l > 0) { if (base64[*b] < 0) break; diff --git a/libarchive/archive_write_set_format_7zip.c b/libarchive/archive_write_set_format_7zip.c index 92a87f74e..e577704b1 100644 --- a/libarchive/archive_write_set_format_7zip.c +++ b/libarchive/archive_write_set_format_7zip.c @@ -164,7 +164,7 @@ struct file { mode_t mode; uint32_t crc32; - int dir:1; + signed int dir:1; }; struct _7zip { diff --git a/libarchive/archive_write_set_format_iso9660.c b/libarchive/archive_write_set_format_iso9660.c index cacbdde7d..e3020cc92 100644 --- a/libarchive/archive_write_set_format_iso9660.c +++ b/libarchive/archive_write_set_format_iso9660.c @@ -289,12 +289,12 @@ struct isoent { struct extr_rec *current; } extr_rec_list; - int virtual:1; + signed int virtual:1; /* If set to one, this file type is a directory. * A convenience flag to be used as * "archive_entry_filetype(isoent->file->entry) == AE_IFDIR". */ - int dir:1; + signed int dir:1; }; struct hardlink { @@ -755,9 +755,9 @@ struct iso9660 { /* Used for making zisofs. */ struct { - int detect_magic:1; - int making:1; - int allzero:1; + signed int detect_magic:1; + signed int making:1; + signed int allzero:1; unsigned char magic_buffer[64]; int magic_cnt; @@ -7801,8 +7801,8 @@ struct zisofs_extract { uint64_t pz_uncompressed_size; size_t uncompressed_buffer_size; - int initialized:1; - int header_passed:1; + signed int initialized:1; + signed int header_passed:1; uint32_t pz_offset; unsigned char *block_pointers; diff --git a/libarchive/archive_write_set_format_xar.c b/libarchive/archive_write_set_format_xar.c index 6efd99553..2e845f866 100644 --- a/libarchive/archive_write_set_format_xar.c +++ b/libarchive/archive_write_set_format_xar.c @@ -212,8 +212,8 @@ struct file { struct heap_data data; struct archive_string script; - int virtual:1; - int dir:1; + signed int virtual:1; + signed int dir:1; }; struct hardlink { @@ -2107,7 +2107,7 @@ file_gen_utility_names(struct archive_write *a, struct file *file) while (len > 0) { size_t ll = len; - if (len > 0 && p[len-1] == '/') { + if (p[len-1] == '/') { p[len-1] = '\0'; len--; } -- 2.47.2