It is easy to get HTTP version comparison wrong. I have added a few comparison operators. To be efficient, they need to be inlined by the compiler so that the following expression does not have to cause HttpVersion(1,0) creation and destruction:

    if (request->http_ver <= HttpVersion(1, 0))

I tried to simplify the constructors and the quality comparison operator to increase our chances that they will be inlined. Unfortunately, I ran into a GCC "feature" which probably explains why we have not written the HttpVersion constructor correctly before.

I added a workaround. Should those #undef lines be moved to some compatibility file? If yes, which one?


-------- proposed commit msg ------------
Simplified HttpVersion initialization and comparison to help compiler inline them.

GCC defines "major()" and "minor()" macros which conflict with HttpVersion member initializations (at least). Undefine those macros if they are present.
----------------------------------------


Thank you,

Alex.
Simplified HttpVersion initialization and comparison to help compiler inline
them.

GCC defines "major()" and "minor()" macros which conflict with HttpVersion
member initializations (at least). Undefine those macros if they are present.


=== modified file 'src/HttpVersion.h'
--- src/HttpVersion.h	2010-08-23 23:25:09 +0000
+++ src/HttpVersion.h	2010-08-23 23:39:51 +0000
@@ -35,35 +35,29 @@
 #ifndef SQUID_HTTPVERSION_H
 #define SQUID_HTTPVERSION_H
 
+// GCC insists on defining "major()" and "minor()" macros in some environments:
+// https://bugzilla.redhat.com/show_bug.cgi?id=130601
+#ifdef major
+#undef major
+#endif
+#ifdef minor
+#undef minor
+#endif
+
 class HttpVersion
 {
 
 public:
-    HttpVersion() {
-        major = 0;
-        minor = 0;
-    }
-
-    HttpVersion(unsigned int aMajor, unsigned int aMinor) {
-        major = aMajor;
-        minor = aMinor;
-    }
-
-    unsigned int major;
-    unsigned int minor;
+    HttpVersion(): major(0), minor(0) {}
+    HttpVersion(unsigned int aMajor, unsigned int aMinor):
+                major(aMajor), minor(aMinor) {}
 
     bool operator==(const HttpVersion& that) const {
-        if (this->major != that.major)
-            return false;
-
-        if (this->minor != that.minor)
-            return false;
-
-        return true;
+        return (this->major == that.major && this->minor == that.minor);
     }
 
     bool operator!=(const HttpVersion& that) const {
-        return ((this->major != that.major) || (this->minor != that.minor));
+        return !(*this == that);
     }
 
     bool operator <(const HttpVersion& that) const {
@@ -83,6 +77,9 @@
     bool operator >=(const HttpVersion& that) const {
         return !(*this < that);
     }
+
+    unsigned int major;
+    unsigned int minor;
 };
 
 #endif /* SQUID_HTTPVERSION_H */

Reply via email to