Avoid abs() of unsigned values.

base/mime_util_xdg.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/mime_util.h"
 
 #include <gtk/gtk.h>
 #include <sys/time.h>
 #include <time.h>
 
@@ skipping 180 matching lines @@
        subdir_iter != subdirs_.end();
        ++subdir_iter) {
     SubDirInfo* info = &info_array_[subdir_iter->second];
     if (MatchesSize(info, size) == 0) {
       icon_path = GetIconPathUnderSubdir(icon_name, subdir_iter->first);
       if (!icon_path.empty())
         return icon_path;
     }
   }
   // Now looking for the mostly matched.
-  int min_delta_seen = 9999;
+  size_t min_delta_seen = 9999;
 
   for (subdir_iter = subdirs_.begin();
        subdir_iter != subdirs_.end();
        ++subdir_iter) {
     SubDirInfo* info = &info_array_[subdir_iter->second];
-    int delta = abs(MatchesSize(info, size));
+    size_t delta = MatchesSize(info, size);
     if (delta < min_delta_seen) {
       FilePath path = GetIconPathUnderSubdir(icon_name, subdir_iter->first);
       if (!path.empty()) {
         min_delta_seen = delta;
         icon_path = path;
       }
     }
   }
 
   if (!icon_path.empty() || !inherits || inherits_ == "")
@@ skipping 100 matching lines @@
       }
     }
   }
 
   file_util::CloseFile(fp);
   return info_array_ != NULL;
 }
 
 size_t IconTheme::MatchesSize(SubDirInfo* info, size_t size) {
   if (info->type == SubDirInfo::Fixed) {
-    return size - info->size;
+    return std::max(size, info->size) - std::min(size, info->size);

[Lei Zhang, 2011/06/13 22:01:15]

can't we just do abs(size - info->size) instead?

[tzik, 2011/06/14 02:52:52]

size - info->size is size_t expression, which is unsigned.
So, semantically abs(size - info->size) should equal to size - info->size.
Yet so stdlib.h in gcc provides abs(int), it is implicitly converted to int.

That makes warning with -Wconversion option in gcc, and makes error in other
implementation of library such as STLport.

So, I want to avoid using abs here.

   } else if (info->type == SubDirInfo::Scalable) {
     if (size >= info->min_size && size <= info->max_size) {

[Lei Zhang, 2011/06/13 22:01:15]

how about we rewrite this block to remove one layer of nested if/else:

if (size < info->min_size)
  return info->min_size - size;
else if (size > info->max_size)
  return size - info->max_size;
else
  return 0;

[tzik, 2011/06/14 02:52:52]

Done.

       return 0;
     } else {
-      return abs(size - info->min_size) < abs(size - info->max_size) ?
-          (size - info->min_size) : (size - info->max_size);
+      if (size < info->min_size)
+        return info->min_size - size;
+      else
+        return size - info->max_size;
     }
   } else {
-    if (size >= info->size - info->threshold &&
+    if (size + info->threshold >= info->size &&

[Lei Zhang, 2011/06/13 22:01:15]

Arg. I'm not even sure the old code was correct. :(

Again, let's rewrite this to be clearer:

if (size + info->threshold < info->size)
  return info->size - info->threshold - size;
else if (size > info->size + info->threshold)
  return size - info->size - info->threshold;
else
  return 0;

[tzik, 2011/06/14 02:52:52]

Done.

         size <= info->size + info->threshold) {
       return 0;
     } else {
-      return abs(size - info->size - info->threshold) <
-          abs(size - info->size + info->threshold)
-          ? size - info->size - info->threshold
-          : size - info->size + info->threshold;
+      if (size < info->size + info->threshold)
+        return size - info->size - info->threshold;
+      else
+        return info->size - info->threshold - size;
     }
   }
 }
 
 std::string IconTheme::ReadLine(FILE* fp) {
   if (!fp)
     return "";
 
   std::string result = "";
   const size_t kBufferSize = 100;
@@ skipping 275 matching lines @@
     } else {
       icon_file = LookupIconInDefaultTheme(icon_names[i], size);
       if (!icon_file.empty())
         return icon_file;
     }
   }
   return FilePath();
 }
 
 }  // namespace mime_util