Static analysis issues

[Naios]

7859f81

Coverity reported some static analysis issues in our TrinityCore check.

format.cc

205
206

CID 1288735 (#1 of 1): Uncaught exception (UNCAUGHT_EXCEPT)
exn_spec_violation: An exception of type fmt::FormatError is thrown but the throw list throw() doesn't allow it to be thrown. This will cause a call to unexpected() which usually calls terminate().
207 void format_error_code(fmt::Writer &out, int error_code,
208                       fmt::StringRef message) FMT_NOEXCEPT {
209  // Report error code making sure that the output fits into
210  // INLINE_BUFFER_SIZE to avoid dynamic memory allocation and potential

618        std::fill_n(out, spec_.width_ - 1, fill);
619        out += spec_.width_ - 1;
620      } else if (spec_.align_ == fmt::ALIGN_CENTER) {

CID 1288736 (#1 of 1): Wrong size argument (SIZEOF_MISMATCH)
suspicious_sizeof: Passing argument 1UL to function (this->writer_) , fill_padding that returns a pointer of type _ZZN3fmt8internal12ArgFormatterIwE10visit_charEiE7CharPtr_25072 is suspicious because a multiple of sizeof (wchar_t) /*4*/ is expected.
621        out = writer_.fill_padding(out, spec_.width_, 1, fill);
622      } else {
623        std::fill_n(out + 1, spec_.width_ - 1, fill);

130template <>
131struct IntChecker<true> {
132  template <typename T>
133  static bool fits_in_int(T value) {

CID 1288737 (#1 of 1): Operands don&#39;t affect result (CONSTANT_EXPRESSION_RESULT)
result_independent_of_operands: value >= -2147483648 /* -2147483647 - 1 */ is always true regardless of the values of its operands. This occurs as the logical first operand of '&&'.
134    return value >= INT_MIN && value <= INT_MAX;
135  }
136};
137

format.h

1172  // Parses argument index and returns corresponding argument.
1173  internal::Arg parse_arg_index(const Char *&s);
1174
1175 public:

CID 1288731 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR)
2. uninit_member: Non-static class member start_ is not initialized in this constructor nor in any functions that it calls.
1176  explicit BasicFormatter(BasicWriter<Char> &w) : writer_(w) {}
1177
1178  BasicWriter<Char> &writer() { return writer_; }

1115class FormatterBase {
1116 private:
1117  ArgList args_;

CID 1288732 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
member_not_init_in_gen_ctor: The compiler-generated constructor for this class does not initialize next_arg_index_.
1118  int next_arg_index_;
1119
1120  // Returns the argument with specified index.
1121  Arg do_get_arg(unsigned arg_index, const char *&error);

1080 public:
1081  // Maximum number of arguments with packed types.
1082  enum { MAX_PACKED_ARGS = 16 };
1083

CID 1288733 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR)
2. uninit_member: Non-static class member args_ is not initialized in this constructor nor in any functions that it calls.
1084  ArgList() : types_(0) {}
1085  ArgList(ULongLong types, const internal::Arg *args)
1086  : types_(types), args_(args) {}
1087

[vitaut]

Thanks for reporting this. Which version did you use? The line numbers don't match with 7859f81.

[Naios]

Oh, sorry my bad the scan is from Jun 26, 2015 where we used fd53bb6.
I thougt the scan included my latest upgrade to 7859f81.

I will update the issue when we get new scan results.

[vitaut]

The first issue is a false positive because FormatError is only thrown for unknown type code in write_int called from format_error_code via operator<<(int) which cannot happen in this case. In fact the type check is optimized away by the GCC.

@Naios Is it possible to suppress false positives in Coverity?

[vitaut]

The second is a false positive too, because fill_padding takes the number of characters as a third argument which doesn't have to be a multiple of sizeof(wchar_t).

[vitaut]

The third case ("Operands don't affect result") is not true in general as can be trivially checked:

// prints 0
fmt::print("{}", IntChecker<true>::fits_in_int(std::numeric_limits<long long>::min()));

Here value is equal -9223372036854775808 on my platform and the expression value >= -2147483648 is false. I guess Coverity makes some incorrect assumptions about types here.

[Naios]

@vitaut You can suppress single false positives for sure.

I'm sorry that it takes so long to update this issue to a newer head but we didn't manage to update the coverity scan filter regex yet.

[vitaut]

No worries, I've submitted the latest version of C++ Format to Coverity myself (https://scan.coverity.com/projects/5671) and I'll try to fix or suppress the warnings as appropriate. One of the warnings in format.h is already resolved in the latest version.

[Naios]

Seems like this is solved

[vitaut]

There are a few more issues reported by Coverity (mostly in tests and Google Mock), but nothing critical and I'll address them separately (#192).

Thanks again for reporting!