[gui] Some fixes to TGNumberEntry

- fix some broken logic
- simplify some functions
- improve safety by adding bounds checks where appropriate

Author: silverweed

gui/gui/src/TGNumberEntry.cxx

@@ -233,19 +233,24 @@ static Bool_t IsGoodChar(char c, TGNumberFormat::EStyle style,
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static char *EliminateGarbage(char *text,
-                              TGNumberFormat::EStyle style,
-                              TGNumberFormat::EAttribute attr)
+/// Copies string `src` into `dst` (up to a length of `dstCap` including the null terminator)
+/// discarding every character that is not a "good character" for the given style and attribute.
+/// \return The length of the resulting string.
+static std::size_t CopyAndEliminateGarbage(char *dst, std::size_t dstCap, const char *src, TGNumberFormat::EStyle style,
+                                           TGNumberFormat::EAttribute attr)
 {
-   if (text == 0) {
+   if (!src)
       return 0;
-   }
-   for (Int_t i = strlen(text) - 1; i >= 0; i--) {
-      if (!IsGoodChar(text[i], style, attr)) {
-         memmove(text + i, text + i + 1, strlen(text) - i);
+
+   std::size_t dstIdx = 0;
+   while (dstIdx < dstCap - 1 && *src) {
+      if (IsGoodChar(*src, style, attr)) {
+         dst[dstIdx++] = *src;
       }
+      ++src;
    }
-   return text;
+   dst[dstIdx] = 0;
+   return dstIdx;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -266,17 +271,9 @@ static Long_t IntStr(const char *text)
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static char *StrInt(char *text, Long_t i, Int_t digits)
+static char *StrInt(char *text, std::size_t textCap, Long_t i, Int_t digits)
 {
-   snprintf(text, 250, "%li", TMath::Abs(i));
-   TString s = text;
-   while (digits > s.Length()) {
-      s = "0" + s;
-   }
-   if (i < 0) {
-      s = "-" + s;
-   }
-   strlcpy(text, (const char *) s, 250);
+   snprintf(text, textCap, "%0*li", digits + (i < 0), i);
    return text;
 }
 
@@ -285,35 +282,45 @@ static char *StrInt(char *text, Long_t i, Int_t digits)
 static TString StringInt(Long_t i, Int_t digits)
 {
    char text[256];
-   StrInt(text, i, digits);
+   StrInt(text, sizeof(text), i, digits);
    return TString(text);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static char *RealToStr(char *text, const RealInfo_t & ri)
+static char *RealToStr(char *text, std::size_t textCap, const RealInfo_t & ri)
 {
    char *p = text;
-   if (text == 0) {
-      return 0;
+   if (!text) {
+      return nullptr;
    }
-   strlcpy(p, "", 256);
+   if (!textCap)
+      return text;
+
+   const auto TextLen = [&p, text, textCap] () -> std::size_t {
+      std::size_t curTextLen = p - text;
+      if (curTextLen >= textCap)
+         return 0;
+      return textCap - curTextLen;
+   };
+
+   strlcpy(p, "", textCap);
    if (ri.fSign < 0) {
-      strlcpy(p, "-", 256);
+      strlcpy(p, "-", textCap);
       p++;
    }
-   StrInt(p, TMath::Abs(ri.fIntNum), 0);
+   StrInt(p, TextLen(), TMath::Abs(ri.fIntNum), 0);
    p += strlen(p);
    if ((ri.fStyle == kRSFrac) || (ri.fStyle == kRSFracExpo)) {
-      strlcpy(p, ".", 256-strlen(text));
+      strlcpy(p, ".", TextLen());
       p++;
-      StrInt(p, TMath::Abs(ri.fFracNum), ri.fFracDigits);
+      StrInt(p, TextLen(), TMath::Abs(ri.fFracNum), ri.fFracDigits);
       p += strlen(p);
    }
    if ((ri.fStyle == kRSExpo) || (ri.fStyle == kRSFracExpo)) {
-      strlcpy(p, "e", 256-strlen(text));
+      strlcpy(p, "e", TextLen());
       p++;
-      StrInt(p, ri.fExpoNum, 0);
+      StrInt(p, TextLen(), ri.fExpoNum, 0);
       p += strlen(p);
    }
    return text;
@@ -436,30 +443,15 @@ static ULong_t HexStrToInt(const char *s)
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static char *IntToHexStr(char *text, ULong_t l)
+static char *IntToHexStr(char *text, std::size_t textCap, ULong_t l)
 {
-   const char *const digits = "0123456789ABCDEF";
-   char buf[64];
-   char *p = buf + 62;
-   // coverity[secure_coding]
-   strcpy(p, "");
-   while (l > 0) {
-      *(--p) = digits[l % 16];
-      l /= 16;
-   }
-   if (!p[0]) {
-      // coverity[secure_coding]
-      strcpy(text, "0");
-   } else {
-      // coverity[secure_coding]
-      strcpy(text, p);
-   }
+   snprintf(text, textCap, "%lX", l);
    return text;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static char *MIntToStr(char *text, Long_t l, Int_t digits)
+static char *MIntToStr(char *text, std::size_t textCap, Long_t l, Int_t digits)
 {
    TString s;
    Int_t base;
@@ -486,13 +478,13 @@ static char *MIntToStr(char *text, Long_t l, Int_t digits)
    if (l < 0) {
       s = "-" + s;
    }
-   strlcpy(text, (const char *) s, 256);
+   strlcpy(text, (const char *) s, textCap);
    return text;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static char *DIntToStr(char *text, Long_t l, Bool_t Sec, char Del)
+static char *DIntToStr(char *text, std::size_t textCap, Long_t l, Bool_t Sec, char Del)
 {
    TString s;
    if (Sec) {
@@ -506,14 +498,14 @@ static char *DIntToStr(char *text, Long_t l, Bool_t Sec, char Del)
    if (l < 0) {
       s = "-" + s;
    }
-   strlcpy(text, (const char *) s, 256);
+   strlcpy(text, (const char *) s, textCap);
    return text;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 /// For kNESMinSecCent
 
-static char *DIntToStr(char *text, Long_t l, char Del, char Del2)
+static char *DIntToStr(char *text, std::size_t textCap, Long_t l, char Del, char Del2)
 {
    TString s;
    s = StringInt(TMath::Abs(l) / 6000, 0) + Del +
@@ -522,7 +514,7 @@ static char *DIntToStr(char *text, Long_t l, char Del, char Del2)
    if (l < 0) {
       s = "-" + s;
    }
-   strlcpy(text, (const char *) s, 256);
+   strlcpy(text, (const char *) s, textCap);
    return text;
 }
 
@@ -574,27 +566,33 @@ static Long_t GetSignificant(Long_t l, Int_t Max)
 
 ////////////////////////////////////////////////////////////////////////////////
 
-static void AppendFracZero(char *text, Int_t digits)
+/// Given a numeric string "xxx.yyy" or "xxx,yyy", makes sure that the fractional
+/// part (if present) always has at least `digits` digits, appending zeroes if needed.
+static void AppendFracZero(char *text, std::size_t textCap, Int_t digits)
 {
-   char *p;
    Int_t found = 0;
-   p = strchr(text, '.');
-   if (p == 0) {
-      p = strchr(text, ',');
+   char *p = strrchr(text, '.');
+   if (!p) {
+      p = strrchr(text, ',');
    }
-   if (p == 0) {
+   if (!p) {
       return;
    }
    p++;
-   for (UInt_t i = 0; i < strlen(p); i++) {
-      if (isdigit(*p)) {
-         found++;
-      }
+   auto pLen = strlen(p);
+   for (UInt_t i = 0; i < pLen; i++) {
+      // NOTE: converting to bool because isdigit doesn't technically necessarily return 0 or 1
+      // (the specs mention it returns "a nonzero value" for positive cases).
+      found += !!isdigit(p[i]);
    }
-   while (found < digits) {
-      // coverity[secure_coding]
-      strcpy(p + strlen(p), "0");
-      found++;
+   auto pOff = p - text;
+   assert(textCap>= pOff + pLen);
+   auto remainingCap = textCap - pOff - pLen;
+   const auto trailingZeroes = std::min<std::size_t>(std::max(0, digits - found), remainingCap - 1);
+   if (trailingZeroes > 0) {
+      memset(p + pLen, '0', trailingZeroes);
+      // ensure the new string is null terminated
+      p[pLen + trailingZeroes] = 0;
    }
 }
 
@@ -646,23 +644,23 @@ static Long_t TranslateToNum(const char *text,
       {
          char buf[256];
          strlcpy(buf, text, sizeof(buf));
-         AppendFracZero(buf, 2);
+         AppendFracZero(buf, sizeof(buf), 2);
          GetNumbers(buf, sign, n1, 12, n2, 2, n3, 0, ".,");
          return sign * (100 * n1 + GetSignificant(n2, 100));
       }
    case TGNumberFormat::kNESRealThree:
       {
          char buf[256];
          strlcpy(buf, text, sizeof(buf));
-         AppendFracZero(buf, 3);
+         AppendFracZero(buf, sizeof(buf), 3);
          GetNumbers(buf, sign, n1, 12, n2, 3, n3, 0, ".,");
          return sign * (1000 * n1 + GetSignificant(n2, 1000));
       }
    case TGNumberFormat::kNESRealFour:
       {
          char buf[256];
          strlcpy(buf, text, sizeof(buf));
-         AppendFracZero(buf, 4);
+         AppendFracZero(buf, sizeof(buf), 4);
          GetNumbers(buf, sign, n1, 12, n2, 4, n3, 0, ".,");
          return sign * (10000 * n1 + GetSignificant(n2, 10000));
       }
@@ -700,40 +698,41 @@ static Long_t TranslateToNum(const char *text,
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Translate a number value to a string.
+/// `textCap` indicates the capacity of `text`.
 
-static char *TranslateToStr(char *text, Long_t l,
+static char *TranslateToStr(char *text, std::size_t textCap, Long_t l,
                             TGNumberFormat::EStyle style, const RealInfo_t & ri)
 {
    switch (style) {
    case TGNumberFormat::kNESInteger:
-      return StrInt(text, l, 0);
+      return StrInt(text, textCap, l, 0);
    case TGNumberFormat::kNESRealOne:
-      return MIntToStr(text, l, 1);
+      return MIntToStr(text, textCap, l, 1);
    case TGNumberFormat::kNESRealTwo:
-      return MIntToStr(text, l, 2);
+      return MIntToStr(text, textCap, l, 2);
    case TGNumberFormat::kNESRealThree:
-      return MIntToStr(text, l, 3);
+      return MIntToStr(text, textCap, l, 3);
    case TGNumberFormat::kNESRealFour:
-      return MIntToStr(text, l, 4);
+      return MIntToStr(text, textCap, l, 4);
    case TGNumberFormat::kNESReal:
-      return RealToStr(text, ri);
+      return RealToStr(text, textCap, ri);
    case TGNumberFormat::kNESDegree:
-      return DIntToStr(text, l, kTRUE, '.');
+      return DIntToStr(text, textCap, l, kTRUE, '.');
    case TGNumberFormat::kNESHourMinSec:
-      return DIntToStr(text, l % (24 * 3600), kTRUE, ':');
+      return DIntToStr(text, textCap, l % (24 * 3600), kTRUE, ':');
    case TGNumberFormat::kNESMinSec:
-      return DIntToStr(text, l, kFALSE, ':');
+      return DIntToStr(text, textCap, l, kFALSE, ':');
    case TGNumberFormat::kNESMinSecCent:
-      return DIntToStr(text, l % (60 * 6000), ':', '.');
+      return DIntToStr(text, textCap, l % (60 * 6000), ':', '.');
    case TGNumberFormat::kNESHourMin:
-      return DIntToStr(text, l % (24 * 60), kFALSE, ':');
+      return DIntToStr(text, textCap, l % (24 * 60), kFALSE, ':');
    case TGNumberFormat::kNESDayMYear:
       {
          TString date =
              StringInt(TMath::Abs(l) % 100, 0) + "/" +
              StringInt((TMath::Abs(l) / 100) % 100, 0) + "/" +
              StringInt(TMath::Abs(l) / 10000, 0);
-         strlcpy(text, (const char *) date, 256);
+         strlcpy(text, (const char *) date, textCap);
          return text;
       }
    case TGNumberFormat::kNESMDayYear:
@@ -742,11 +741,11 @@ static char *TranslateToStr(char *text, Long_t l,
              StringInt((TMath::Abs(l) / 100) % 100, 0) + "/" +
              StringInt(TMath::Abs(l) % 100, 0) + "/" +
              StringInt(TMath::Abs(l) / 10000, 0);
-         strlcpy(text, (const char *) date, 256);
+         strlcpy(text, (const char *) date, textCap);
          return text;
       }
    case TGNumberFormat::kNESHex:
-      return IntToHexStr(text, (ULong_t) l);
+      return IntToHexStr(text, textCap, (ULong_t) l);
    }
    return 0;
 }
@@ -1200,9 +1199,9 @@ void TGNumberEntryField::SetIntNumber(Long_t val, Bool_t emit)
    char text[256];
    RealInfo_t ri;
    if (fNumStyle == kNESReal) {
-      TranslateToStr(text, val, kNESInteger, ri);
+      TranslateToStr(text, sizeof(text), val, kNESInteger, ri);
    } else {
-      TranslateToStr(text, val, fNumStyle, ri);
+      TranslateToStr(text, sizeof(text), val, fNumStyle, ri);
    }
    SetText(text, emit);
 }
@@ -1265,8 +1264,7 @@ void TGNumberEntryField::SetHexNumber(ULong_t val, Bool_t emit)
 void TGNumberEntryField::SetText(const char *text, Bool_t emit)
 {
    char buf[256];
-   strlcpy(buf, text, sizeof(buf));
-   EliminateGarbage(buf, fNumStyle, fNumAttr);
+   CopyAndEliminateGarbage(buf, sizeof(buf), text, fNumStyle, fNumAttr);
    TGTextEntry::SetText(buf, emit);
    fNeedsVerification = kFALSE;
 }
@@ -1617,7 +1615,7 @@ void TGNumberEntryField::IncreaseNumber(EStepSize step,
       SetIntNumber(l);
    } else {
       char buf[256];
-      RealToStr(buf, ri);
+      RealToStr(buf, sizeof(buf), ri);
       SetText(buf);
    }
 }
@@ -2253,7 +2251,7 @@ void TGNumberEntry::SavePrimitive(std::ostream &out, Option_t *option /*= ""*/)
       break;
    case kNESHex: {
       char hexstr[256];
-      IntToHexStr(hexstr, GetHexNumber());
+      IntToHexStr(hexstr, sizeof(hexstr), GetHexNumber());
       out << "0x" << hexstr << "U, " << digits << ", " << WidgetId() << ",(TGNumberFormat::EStyle) " << GetNumStyle();
       break;
    }
@@ -2323,7 +2321,7 @@ void TGNumberEntryField::SavePrimitive(std::ostream &out, Option_t *option /*= "
    case kNESMDayYear: out << yy << mm << dd << ",(TGNumberFormat::EStyle) " << GetNumStyle(); break;
    case kNESHex: {
       char hexstr[256];
-      IntToHexStr(hexstr, GetHexNumber());
+      IntToHexStr(hexstr, sizeof(hexstr), GetHexNumber());
       out << "0x" << hexstr << "U, (TGNumberFormat::EStyle) " << GetNumStyle();
       break;
    }