Skip to content

Commit

Permalink
Fix high certainty warnings from PVS-studio (google#5115)
Browse files Browse the repository at this point in the history
* Fix high certainty warnings from PVS-studio

- Introduced FLATBUFFERS_ATTRIBUTE macro to use [[attribute]] if modern C++ compiler used

* Update the note about __cplusplus usage in the MSVC
  • Loading branch information
vglavnyy authored and aardappel committed Jan 17, 2019
1 parent bbfd12e commit d449316
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 25 deletions.
18 changes: 18 additions & 0 deletions include/flatbuffers/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@
// Clang 3.4 and later implement all of the ISO C++ 2014 standard.
// http://clang.llvm.org/cxx_status.html

// Note the MSVC value '__cplusplus' may be incorrect:
// The '__cplusplus' predefined macro in the MSVC stuck at the value 199711L,
// indicating (erroneously!) that the compiler conformed to the C++98 Standard.
// This value should be correct starting from MSVC2017-15.7-Preview-3.
// The '__cplusplus' will be valid only if MSVC2017-15.7-P3 and the `/Zc:__cplusplus` switch is set.
// Workaround (for details see MSDN):
// Use the _MSC_VER and _MSVC_LANG definition instead of the __cplusplus for compatibility.
// The _MSVC_LANG macro reports the Standard version regardless of the '/Zc:__cplusplus' switch.

/// @cond FLATBUFFERS_INTERNAL
#if __cplusplus <= 199711L && \
(!defined(_MSC_VER) || _MSC_VER < 1600) && \
Expand Down Expand Up @@ -224,6 +233,15 @@ template<typename T> FLATBUFFERS_CONSTEXPR inline bool IsConstTrue(T t) {
return !!t;
}

// Enable of std:c++17 or higher.
#if (defined(__cplusplus) && (__cplusplus >= 201703L)) || \
(defined(_MSVC_LANG) && (_MSVC_LANG >= 201703L))
// All attributes unknown to an implementation are ignored without causing an error.
#define FLATBUFFERS_ATTRIBUTE(attr) // [[attr]] - will be enabled in a future release
#else
#define FLATBUFFERS_ATTRIBUTE(attr)
#endif

/// @endcond

/// @file
Expand Down
11 changes: 6 additions & 5 deletions include/flatbuffers/flatbuffers.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ template<typename T> class Vector {
uoffset_t size() const { return EndianScalar(length_); }

// Deprecated: use size(). Here for backwards compatibility.
FLATBUFFERS_ATTRIBUTE(deprecated("use size() instead"))
uoffset_t Length() const { return size(); }

typedef typename IndirectHelper<T>::return_type return_type;
Expand Down Expand Up @@ -996,8 +997,8 @@ class FlatBufferBuilder {
/// @warning Do NOT attempt to use this FlatBufferBuilder afterwards!
/// @return A `FlatBuffer` that owns the buffer and its allocator and
/// behaves similar to a `unique_ptr` with a deleter.
/// Deprecated: use Release() instead
DetachedBuffer ReleaseBufferPointer() {
FLATBUFFERS_ATTRIBUTE(deprecated("use Release() instead")) DetachedBuffer
ReleaseBufferPointer() {
Finished();
return buf_.release();
}
Expand Down Expand Up @@ -1205,7 +1206,7 @@ class FlatBufferBuilder {
auto vt_offset_ptr = reinterpret_cast<uoffset_t *>(it);
auto vt2 = reinterpret_cast<voffset_t *>(buf_.data_at(*vt_offset_ptr));
auto vt2_size = *vt2;
if (vt1_size != vt2_size || memcmp(vt2, vt1, vt1_size)) continue;
if (vt1_size != vt2_size || 0 != memcmp(vt2, vt1, vt1_size)) continue;
vt_use = *vt_offset_ptr;
buf_.pop(GetSize() - vtableoffsetloc);
break;
Expand All @@ -1226,7 +1227,7 @@ class FlatBufferBuilder {
return vtableoffsetloc;
}

// DEPRECATED: call the version above instead.
FLATBUFFERS_ATTRIBUTE(deprecated("call the version above instead"))
uoffset_t EndTable(uoffset_t start, voffset_t /*numfields*/) {
return EndTable(start);
}
Expand Down Expand Up @@ -2114,7 +2115,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS {
if (!Verify<uoffset_t>(start)) return 0;
auto o = ReadScalar<uoffset_t>(buf_ + start);
// May not point to itself.
Check(o != 0);
if (!Check(o != 0)) return 0;
// Can't wrap around / buffers are max 2GB.
if (!Check(static_cast<soffset_t>(o) >= 0)) return 0;
// Must be inside the buffer to create a pointer from it (pointer outside
Expand Down
4 changes: 2 additions & 2 deletions include/flatbuffers/idl.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ inline size_t InlineAlignment(const Type &type) {

struct EnumVal {
EnumVal(const std::string &_name, int64_t _val) : name(_name), value(_val) {}
EnumVal(){};
EnumVal() : value(0){};

Offset<reflection::EnumVal> Serialize(FlatBufferBuilder *builder, const Parser &parser) const;

Expand Down Expand Up @@ -672,7 +672,7 @@ class Parser : public ParserState {
bool Deserialize(const reflection::Schema* schema);

Type* DeserializeType(const reflection::Type* type);

// Checks that the schema represented by this parser is a safe evolution
// of the schema provided. Returns non-empty error on any problems.
std::string ConformTo(const Parser &base);
Expand Down
2 changes: 1 addition & 1 deletion include/flatbuffers/registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class Registry {
return DetachedBuffer();
}
// We have a valid FlatBuffer. Detach it from the builder and return.
return parser.builder_.ReleaseBufferPointer();
return parser.builder_.Release();
}

// Modify any parsing / output options used by the other functions.
Expand Down
2 changes: 1 addition & 1 deletion include/flatbuffers/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ inline bool is_digit(char c) { return check_in_range(c, '0', '9'); }

inline bool is_xdigit(char c) {
// Replace by look-up table.
return is_digit(c) | check_in_range(c & 0xDF, 'a' & 0xDF, 'f' & 0xDF);
return is_digit(c) || check_in_range(c & 0xDF, 'a' & 0xDF, 'f' & 0xDF);
}

// Case-insensitive isalnum
Expand Down
3 changes: 2 additions & 1 deletion samples/sample_bfbs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,15 @@ int main(int /*argc*/, const char * /*argv*/[]) {
flatbuffers::Parser parser1;
ok = parser1.Parse(schema_file.c_str(), include_directories);
assert(ok);

// inizialize parser by deserializing bfbs schema
flatbuffers::Parser parser2;
ok = parser2.Deserialize((uint8_t *)bfbs_file.c_str(), bfbs_file.length());
assert(ok);

// parse json in parser from fbs and bfbs
ok = parser1.Parse(json_file.c_str(), include_directories);
assert(ok);
ok = parser2.Parse(json_file.c_str(), include_directories);
assert(ok);

Expand Down
10 changes: 6 additions & 4 deletions src/idl_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,10 @@ CheckedError Parser::Next() {
}
cursor_ += 2;
break;
} else {
// fall thru
}
// fall thru
FLATBUFFERS_ATTRIBUTE(fallthrough);
default:
const auto has_sign = (c == '+') || (c == '-');
// '-'/'+' and following identifier - can be a predefined constant like:
Expand Down Expand Up @@ -2752,7 +2754,7 @@ bool StructDef::Deserialize(Parser &parser, const reflection::Object *object) {
minalign = object->minalign();
predecl = false;
sortbysize = attributes.Lookup("original_order") == nullptr && !fixed;
std::vector<uoffset_t> indexes =
std::vector<uoffset_t> indexes =
std::vector<uoffset_t>(object->fields()->Length());
for (uoffset_t i = 0; i < object->fields()->Length(); i++)
indexes[object->fields()->Get(i)->id()] = i;
Expand All @@ -2768,7 +2770,7 @@ bool StructDef::Deserialize(Parser &parser, const reflection::Object *object) {
// Recompute padding since that's currently not serialized.
auto size = InlineSize(field_def->value.type);
auto next_field =
i + 1 < indexes.size()
i + 1 < indexes.size()
? object->fields()->Get(indexes[i+1])
: nullptr;
bytesize += size;
Expand Down Expand Up @@ -3036,7 +3038,7 @@ bool Parser::Deserialize(const uint8_t *buf, const size_t size) {
return false;
else
size_prefixed = true;
}
}
auto verify_fn = size_prefixed ? &reflection::VerifySizePrefixedSchemaBuffer
: &reflection::VerifySchemaBuffer;
if (!verify_fn(verifier)) {
Expand Down
24 changes: 13 additions & 11 deletions tests/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ flatbuffers::DetachedBuffer CreateFlatBufferTest(std::string &buffer) {
reinterpret_cast<const char *>(builder.GetBufferPointer());
buffer.assign(bufferpointer, bufferpointer + builder.GetSize());

return builder.ReleaseBufferPointer();
return builder.Release();
}

// example of accessing a buffer loaded in memory:
Expand Down Expand Up @@ -2032,6 +2032,7 @@ void UnionVectorTest() {
TEST_EQ(cts->GetEnum<Character>(4) == Character_Unused, true);

auto rapunzel = movie->main_character_as_Rapunzel();
TEST_NOTNULL(rapunzel);
TEST_EQ(rapunzel->hair_length(), 6);

auto cs = movie->characters();
Expand Down Expand Up @@ -2309,16 +2310,17 @@ void TypeAliasesTest() {
TEST_EQ(ta->u64(), flatbuffers::numeric_limits<uint64_t>::max());
TEST_EQ(ta->f32(), 2.3f);
TEST_EQ(ta->f64(), 2.3);
TEST_EQ(sizeof(ta->i8()), 1);
TEST_EQ(sizeof(ta->i16()), 2);
TEST_EQ(sizeof(ta->i32()), 4);
TEST_EQ(sizeof(ta->i64()), 8);
TEST_EQ(sizeof(ta->u8()), 1);
TEST_EQ(sizeof(ta->u16()), 2);
TEST_EQ(sizeof(ta->u32()), 4);
TEST_EQ(sizeof(ta->u64()), 8);
TEST_EQ(sizeof(ta->f32()), 4);
TEST_EQ(sizeof(ta->f64()), 8);
using namespace flatbuffers; // is_same
static_assert(is_same<decltype(ta->i8()), int8_t>::value, "invalid type");
static_assert(is_same<decltype(ta->i16()), int16_t>::value, "invalid type");
static_assert(is_same<decltype(ta->i32()), int32_t>::value, "invalid type");
static_assert(is_same<decltype(ta->i64()), int64_t>::value, "invalid type");
static_assert(is_same<decltype(ta->u8()), uint8_t>::value, "invalid type");
static_assert(is_same<decltype(ta->u16()), uint16_t>::value, "invalid type");
static_assert(is_same<decltype(ta->u32()), uint32_t>::value, "invalid type");
static_assert(is_same<decltype(ta->u64()), uint64_t>::value, "invalid type");
static_assert(is_same<decltype(ta->f32()), float>::value, "invalid type");
static_assert(is_same<decltype(ta->f64()), double>::value, "invalid type");
}

void EndianSwapTest() {
Expand Down

0 comments on commit d449316

Please sign in to comment.