]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
Multiple code fixes and optimizations
authorMartin Matuska <martin@matuska.org>
Tue, 14 Jan 2020 13:40:11 +0000 (14:40 +0100)
committerMartin Matuska <martin@matuska.org>
Tue, 14 Jan 2020 15:59:55 +0000 (16:59 +0100)
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
libarchive/archive_read_disk_posix.c
libarchive/archive_read_support_filter_uu.c
libarchive/archive_read_support_format_rar.c
libarchive/archive_read_support_format_rar5.c
libarchive/archive_read_support_format_xar.c
libarchive/archive_write_set_format_7zip.c
libarchive/archive_write_set_format_iso9660.c
libarchive/archive_write_set_format_xar.c

index 72c644e6079b06946a210e2afe033853a1c22d93..a15e98c284256dff04de082982d4ea1c41eeef65 100644 (file)
@@ -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) {
index f7c96bffa91c2b86053d1a765cc4422af89a7dbe..52fec7bb42c8a715eb5e5d017aa09bbf311e117d 100644 (file)
@@ -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,
index 641297990d26107770a7be7029ada33ca2c67a53..67ddffb06943633fbf9737417bcfc2029e54d9d4 100644 (file)
@@ -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;
index fe5903b6456fcb4d2340d1e43b1d1d3110fe3fa7..98efbb1a6c4a3b6bdb6c65703217b7cd0c3c5f06 100644 (file)
 #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)
index ce38b1fc990f360cea0aef8cbff0abf0164fcf21..6e91eab851bb082bd5c1d08e479068f5b1d8eb61 100644 (file)
@@ -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. */
index 34253a52fb75332415f2ce286372c71ba3dd9a96..7f8be398c7a2643960ab6a0cb4838567db0176b2 100644 (file)
@@ -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;
index 92a87f74e62507c0d5604a9a2b9060a4d713a1d4..e577704b16ad466ea9ed54e8ac1bb6425c50de2b 100644 (file)
@@ -164,7 +164,7 @@ struct file {
        mode_t                   mode;
        uint32_t                 crc32;
 
-       int                      dir:1;
+       signed int               dir:1;
 };
 
 struct _7zip {
index cacbdde7dcb0faf44c34aaad3539fab1bceed418..e3020cc92ea76ddd5a0eae2d9a47a07df2d50a40 100644 (file)
@@ -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;
index 6efd99553a27828d17035d529aa709c6017d4652..2e845f8668a61293a3eb72c4d9c0566b01d13962 100644 (file)
@@ -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--;
                }