Merge pull request #1010 from intel/push-2026-04-06

opcm · web-flow · commit abb6bce87cc2 · 2026-04-09T13:58:55.000+02:00
Push 2026 04 06
diff --git a/AGENTS.md b/AGENTS.md
@@ -0,0 +1,69 @@
+# Agent-Assisted Code Improvements
+
+This document tracks code improvements made with the assistance of AI agents.
+
+> **Note:** For compilation instructions, see [README.md](README.md).
+
+## MSVC Compilation Error Fix - numeric_limits::max() Macro Conflict
+
+**Commit:** 2e387a11  
+**Date:** 2026-04-06  
+**File:** `src/pcm-sensor-server.cpp`
+
+### Issue
+
+MSVC compilation error caused by macro conflict with `std::numeric_limits<T>::max()`
+
+The error occurred at two locations in the argument parsing code (the `-p` port and `-D` debug level options):
+
+```cpp
+// Original code (problematic)
+if ( val > std::numeric_limits<unsigned short>::max() )
+    throw std::out_of_range( "port out of range" );
+```
+
+### Root Cause
+
+Windows headers (particularly `windows.h` and related headers) often define `max` and `min` as preprocessor macros:
+
+```cpp
+#define max(a,b) (((a) > (b)) ? (a) : (b))
+```
+
+When the preprocessor encounters `std::numeric_limits<unsigned short>::max()`, the `max()` part gets expanded as a macro, resulting in malformed code that fails to compile.
+
+### Solution
+
+Wrap the function name in parentheses to prevent macro expansion:
+
+```cpp
+// Fixed code
+if ( val > (std::numeric_limits<unsigned short>::max)() )
+    throw std::out_of_range( "port out of range" );
+```
+
+By adding parentheses around `max`, the preprocessor no longer recognizes it as a macro invocation, allowing the actual member function to be called.
+
+### Technical Details
+
+- **Affected code:** Port validation (`-p` option) and debug level validation (`-D` option)
+- **Context:** Command-line argument parsing for `-p` (port) and `-D` (debug level) options
+- **Alternative solutions:**
+  1. `#undef max` before use (not recommended, affects entire translation unit)
+  2. Define `NOMINMAX` before including Windows headers (project-wide change)
+  3. Use `(std::numeric_limits<T>::max)()` (chosen solution - local and surgical)
+
+### Why This Approach
+
+The parentheses-wrapping approach is preferred because:
+1. **Minimal impact:** Only affects the specific call sites
+2. **No side effects:** Doesn't disable macros for other code that might depend on them
+3. **Clear intent:** Documents that macro expansion is being avoided
+4. **Standard practice:** Commonly used pattern in cross-platform C++ code
+
+### Best Practices
+
+When writing portable C++ code that may be compiled with Windows headers:
+- Always wrap standard library function names like `min`, `max` in parentheses when calling them
+- Consider using `(std::min)(a, b)` and `(std::max)(a, b)` for function calls, and `(std::numeric_limits<T>::max)()` for `numeric_limits`
+- Be aware that macros from platform headers can interfere with identically-named functions
diff --git a/src/debug.h b/src/debug.h
@@ -6,6 +6,9 @@
 #include <sstream>
 #include <iomanip>
 #include <iostream>
+#include <utility>
+#include <ctime>
+#include "types.h"
 
 #ifdef _MSC_VER
 #include <BaseTsd.h>
@@ -15,6 +18,9 @@
 
 namespace pcm {
 
+// Forward declaration
+std::pair<tm, uint64> pcm_localtime();
+
 namespace debug {
     extern int currentDebugLevel;
 
@@ -32,10 +38,10 @@ namespace debug {
     template<typename LVL, typename PF, typename F, typename L, typename... Args>
     void dyn_debug_output( std::ostream& out, LVL level, PF pretty_function, F file, L line, Args... args ) {
         std::stringstream ss;
-        auto now = time(nullptr);
+        auto tt = pcm_localtime();
         ss << "DBG(" << std::dec << level << "): File '" << file << "', line '" << std::dec << line << "' :\n";
         ss << "DBG(" << std::dec << level << "): " << pretty_function << ":\n";
-        ss << "DBG(" << std::dec << level << ") " << std::put_time( localtime(&now), "%F_%T: " ); // Next code line will continue printing on this output line
+        ss << "DBG(" << std::dec << level << ") " << std::put_time( &tt.first, "%F_%T: " ); // Next code line will continue printing on this output line
         dyn_debug_output_helper( ss, args... );
         out << ss.str() << std::flush;
     }
diff --git a/src/pcm-msr.cpp b/src/pcm-msr.cpp
@@ -141,7 +141,9 @@ int mainThrows(int argc, char * argv[])
         printf("SDL330 ERROR: Symlink/reparse point detected at '%s' (skipping write)\n", outfile.c_str());
         outflag = false;
       } else {
-        ofile = fopen(outfile.c_str(),"w");
+        if (fopen_s(&ofile, outfile.c_str(), "w") != 0) {
+          ofile = NULL;
+        }
       }
 #else
       // SDL330: Use O_NOFOLLOW to reject symlinks
diff --git a/src/pcm-sensor-server.cpp b/src/pcm-sensor-server.cpp
@@ -29,9 +29,9 @@ constexpr unsigned int DEFAULT_HTTPS_PORT = DEFAULT_HTTP_PORT;
 typedef SOCKET socket_t;
 #define SHUT_RDWR SD_BOTH
 #define MSG_NOSIGNAL 0
-// errno values
-#define EAGAIN WSAEWOULDBLOCK
-#define EWOULDBLOCK WSAEWOULDBLOCK
+// PCM errno values mapped to Windows socket errors
+#define PCM_EAGAIN WSAEWOULDBLOCK
+#define PCM_EWOULDBLOCK WSAEWOULDBLOCK
 inline int close(SOCKET s) { return closesocket(s); }
 #else
 #include <unistd.h>
@@ -43,12 +43,16 @@ inline int close(SOCKET s) { return closesocket(s); }
 #include <sched.h>
 typedef int socket_t;
 #define INVALID_SOCKET (-1)
+// PCM errno values mapped to POSIX errors
+#define PCM_EAGAIN EAGAIN
+#define PCM_EWOULDBLOCK EWOULDBLOCK
 #define SOCKET_ERROR (-1)
 #endif
 
 #include <cstring>
 #include <fstream>
 #include <ctime>
+#include <limits>
 #include <vector>
 #include <unordered_map>
 
@@ -129,10 +133,17 @@ class datetime {
     public:
         datetime() {
             std::time_t t = std::time( nullptr );
+#ifdef _MSC_VER
+            std::tm tm_buf;
+            if (gmtime_s(&tm_buf, &t) != 0)
+                throw std::runtime_error("gmtime_s failed");
+            now = tm_buf;
+#else
             const auto gt = std::gmtime( &t );
             if (gt == nullptr)
                 throw std::runtime_error("std::gmtime returned nullptr");
             now = *gt;
+#endif
         }
         datetime( std::tm t ) : now( t ) {}
         ~datetime() = default;
@@ -184,9 +195,19 @@ class date {
     public:
         void printDate( std::ostream& os ) const {
             char buf[64];
+#ifdef _MSC_VER
+            std::tm tm_buf;
+            if (localtime_s(&tm_buf, &now) != 0)
+                throw std::runtime_error("localtime_s failed");
+            if (std::strftime(buf, 64, "%F", &tm_buf) == 0)
+                throw std::runtime_error("Error writing date to buffer, too small?");
+#else
             const auto t = std::localtime(&now);
-            assert(t);
-            std::strftime( buf, 64, "%F", t);
+            if (t == nullptr)
+                throw std::runtime_error("std::localtime returned nullptr");
+            if (std::strftime(buf, 64, "%F", t) == 0)
+                throw std::runtime_error("Error writing date to buffer, too small?");
+#endif
             os << buf;
         }
 
@@ -1274,7 +1295,7 @@ class basic_socketbuf : public std::basic_streambuf<CharT> {
                         switch ( sslError ) {
                             case SSL_ERROR_WANT_READ:
                                 DBG( 3, "SSL_ERROR_WANT_READ: Errno = ", errno, ", strerror(errno): ", strerror(errno) );
-                                if ( errno == EAGAIN || errno == EWOULDBLOCK ) {
+                                if ( errno == PCM_EAGAIN || errno == PCM_EWOULDBLOCK ) {
                                     DBG( 3, dbg, "Most likely the set timeout, so aborting..." );
                                     close();
                                     Base::setg( nullptr, nullptr, nullptr );
@@ -1288,7 +1309,7 @@ class basic_socketbuf : public std::basic_streambuf<CharT> {
                                 break;
                             case SSL_ERROR_SYSCALL:
                                 DBG( 3, "SSL_ERROR_SYSCALL: Errno = ", errno );
-                                if ( errno == EAGAIN || errno == EWOULDBLOCK ) {
+                                if ( errno == PCM_EAGAIN || errno == PCM_EWOULDBLOCK ) {
                                     DBG( 3, dbg, "Most likely the set timeout, so aborting..." );
                                     close();
                                     Base::setg( nullptr, nullptr, nullptr );
@@ -2128,7 +2149,16 @@ struct URL {
                                 DBG( 3, "number of characters processed: ", pos );
                             } catch ( std::out_of_range& e ) {
                                 DBG( 3, "out_of_range exception caught in stoull: ", e.what() );
+#ifdef _MSC_VER
+                                char errbuf[256];
+                                if (strerror_s(errbuf, sizeof(errbuf), errno) == 0) {
+                                    DBG( 3, "errno: ", errno, ", strerror(errno): ", errbuf );
+                                } else {
+                                    DBG( 3, "errno: ", errno, ", strerror(errno): (error converting)" );
+                                }
+#else
                                 DBG( 3, "errno: ", errno, ", strerror(errno): ", strerror(errno) );
+#endif
                             }
                         }
                         if ( port >= 65536 )
@@ -3173,9 +3203,10 @@ class HTTPConnection : public Work {
             }
 
             // Do processing of the request here
-            if (*callbackList_[request.method()])
-                (*callbackList_[request.method()])( hs_, request, response );
-            else {
+            auto callback = callbackList_[request.method()];
+            if ( callback ) {
+                (*callback)( hs_, request, response );
+            } else {
                 std::string body( "501 Not Implemented." );
                 body += " Method \"" + HTTPMethodProperties::getMethodAsString(request.method()) + "\" is not implemented (yet).";
                 response.createResponse( TextPlain, body, RC_501_NotImplemented );
@@ -3511,8 +3542,8 @@ void HTTPServer::run() {
         HTTPConnection* connection = nullptr;
         try {
             connection = new HTTPConnection( this, clientSocketFD, clientAddress, callbackList_ );
-        } catch ( std::exception& e ) {
-            DBG( 3, "Exception caught while creating a HTTPConnection: " );
+        } catch ( const std::exception& e ) {
+            DBG( 3, "Exception caught while creating a HTTPConnection: ", e.what() );
             deleteAndNullify( connection );
             DBG( 3, "close clientsocketFD" );
             ::close( clientSocketFD );
@@ -4101,11 +4132,22 @@ int mainThrows(int argc, char * argv[]) {
             else if ( check_argument_equals( argv[i], {"-p"} ) )
             {
                 if ( (++i) < argc ) {
-                    std::stringstream ss( argv[i] );
                     try {
-                        ss >> port;
-                    } catch( std::exception& e ) {
-                        std::cerr << "main: port number is not an unsigned short!\n";
+                        std::size_t pos = 0;
+                        unsigned long val = std::stoul( argv[i], &pos );
+                        if ( pos != std::strlen( argv[i] ) )
+                            throw std::invalid_argument( "invalid port" );
+                        if ( val > (std::numeric_limits<unsigned short>::max)() )
+                            throw std::out_of_range( "port out of range" );
+                        port = static_cast<unsigned short>( val );
+                    } catch( const std::invalid_argument& e ) {
+                        std::cerr << "main: invalid port argument '" << argv[i] << "': " << e.what() << "\n";
+                        ::exit( 2 );
+                    } catch( const std::out_of_range& e ) {
+                        std::cerr << "main: port argument out of range '" << argv[i] << "': " << e.what() << "\n";
+                        ::exit( 2 );
+                    } catch( const std::exception& e ) {
+                        std::cerr << "main: failed to parse port argument '" << argv[i] << "': " << e.what() << "\n";
                         ::exit( 2 );
                     }
                 } else {
@@ -4144,11 +4186,19 @@ int mainThrows(int argc, char * argv[]) {
             else if ( check_argument_equals( argv[i], {"-D", "--debug"} ) )
             {
                 if ( (++i) < argc ) {
-                    std::stringstream ss( argv[i] );
                     try {
-                        ss >> debug_level;
-                    } catch( std::exception& e ) {
-                        std::cerr << "main: debug level is not an unsigned short!\n";
+                        std::size_t pos = 0;
+                        unsigned long val = std::stoul( argv[i], &pos );
+                        if ( pos != std::strlen( argv[i] ) )
+                            throw std::invalid_argument( "invalid debug level" );
+                        if ( val > (std::numeric_limits<unsigned short>::max)() )
+                            throw std::out_of_range( "debug level out of range" );
+                        debug_level = static_cast<unsigned short>( val );
+                    } catch ( const std::invalid_argument& e ) {
+                        std::cerr << "main: invalid debug level '" << argv[i] << "': " << e.what() << "\n";
+                        ::exit( 2 );
+                    } catch ( const std::out_of_range& e ) {
+                        std::cerr << "main: debug level '" << argv[i] << "' is out of range: " << e.what() << "\n";
                         ::exit( 2 );
                     }
                 } else {
