Avoid abs() of unsigned values.

base/mime_util_xdg.cc

Index: base/mime_util_xdg.cc
diff --git a/base/mime_util_xdg.cc b/base/mime_util_xdg.cc
index e510b689457b8303ddb9b2e919b647976c37b83d..c92043c0e2c5480fa3d8df0b7dfaa1fd04bf33c9 100644
--- a/base/mime_util_xdg.cc
+++ b/base/mime_util_xdg.cc
@@ -198,13 +198,13 @@ FilePath IconTheme::GetIconPath(const std::string& icon_name, int size,
     }
   }
   // 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()) {
@@ -325,23 +325,25 @@ bool IconTheme::LoadIndexTheme(const FilePath& file) {
 
 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;
     }
   }
 }