Unspecified behavior: shift operator '>>' on signed integer #744

Closed
bagder opened this Issue Mar 31, 2016 · 1 comment

Projects

None yet

2 participants

@bagder
Member
bagder commented Mar 31, 2016

Alexis La Goutte (@alagoutte) ran PVS Studio on the curl code. One of the warnings is

V610 Unspecified behavior. Check the shift operator '>>'. The left operand 'err' is negative. strerror.c 1082

This code (https://github.com/curl/curl/blob/master/lib/strerror.c#L1082) comes from commit 995c600 but I can't figure out why it is split in two sprintf formats like that. So I ask you here. I propose a fix like this to avoid the warning/problem:

--- a/lib/strerror.c
+++ b/lib/strerror.c
@@ -1074,14 +1074,14 @@ const char *Curl_sspi_strerror (struct connectdata *conn, int err)

   if(err == SEC_E_OK)
     strncpy(outbuf, txt, outmax);
   else if(err == SEC_E_ILLEGAL_MESSAGE)
     snprintf(outbuf, outmax,
-             "SEC_E_ILLEGAL_MESSAGE (0x%04X%04X) - This error usually occurs "
+             "SEC_E_ILLEGAL_MESSAGE (0x%08x) - This error usually occurs "
              "when a fatal SSL/TLS alert is received (e.g. handshake failed). "
              "More detail may be available in the Windows System event log.",
-             (err >> 16) & 0xffff, err & 0xffff);
+             err);
   else {
     str = txtbuf;
     snprintf(txtbuf, sizeof(txtbuf), "%s (0x%04X%04X)",
              txt, (err >> 16) & 0xffff, err & 0xffff);
     txtbuf[sizeof(txtbuf)-1] = '\0';
@jay jay was assigned by bagder Mar 31, 2016
@jay jay added a commit that referenced this issue Apr 1, 2016
@jay jay strerror: don't bit shift a signed integer
Bug: #744
Reported-by: Alexis La Goutte
7c314fd
@jay
Member
jay commented Apr 1, 2016

I'm speculating at this point but it's likely that when I added that message I just copied the way it's done a few lines below as seen in your diff. I can't see any good reason for it so I've changed both, thanks!

Landed in 7c314fd.

@jay jay closed this Apr 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment