Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(306)

Unified Diff: mojo/system/options_validation.h

Issue 414393002: Convert verification of options structs to use the new user pointer handling (see r285350). (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: temporarily disable part of OptionsValidationTest.InvalidDeath Created 6 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « mojo/system/message_pipe_dispatcher.cc ('k') | mojo/system/options_validation_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: mojo/system/options_validation.h
diff --git a/mojo/system/options_validation.h b/mojo/system/options_validation.h
index 04514adf752e1274d015623495393162b5675f4b..90c15685b971e78eff4afb5cf1dbf5c7f4df69c8 100644
--- a/mojo/system/options_validation.h
+++ b/mojo/system/options_validation.h
@@ -14,108 +14,88 @@
#include <stddef.h>
#include <stdint.h>
+#include <algorithm>
+
+#include "base/logging.h"
#include "base/macros.h"
#include "mojo/public/c/system/types.h"
+#include "mojo/system/constants.h"
#include "mojo/system/memory.h"
#include "mojo/system/system_impl_export.h"
namespace mojo {
namespace system {
-// Checks that |buffer| appears to contain a valid Options struct, namely
-// properly aligned and with a |struct_size| field (which must the first field
-// of the struct and be a |uint32_t|) containing a plausible size.
-template <class Options>
-bool IsOptionsStructPointerAndSizeValid(const void* buffer) {
- COMPILE_ASSERT(offsetof(Options, struct_size) == 0,
- Options_struct_size_not_first_member);
- // TODO(vtl): With C++11, use |sizeof(Options::struct_size)| instead.
- COMPILE_ASSERT(sizeof(static_cast<const Options*>(buffer)->struct_size) ==
- sizeof(uint32_t),
- Options_struct_size_not_32_bits);
-
- // Note: Use |MOJO_ALIGNOF()| here to match the exact macro used in the
- // declaration of Options structs.
- if (!internal::VerifyUserPointerHelper<sizeof(uint32_t),
- MOJO_ALIGNOF(Options)>(buffer))
- return false;
-
- return static_cast<const Options*>(buffer)->struct_size >= sizeof(uint32_t);
-}
-
-// Checks that the Options struct in |buffer| has a member with the given offset
-// and size. This may be called only if |IsOptionsStructPointerAndSizeValid()|
-// returned true.
-//
-// You may want to use the macro |HAS_OPTIONS_STRUCT_MEMBER()| instead.
-template <class Options, size_t offset, size_t size>
-bool HasOptionsStructMember(const void* buffer) {
- // We assume that |offset| and |size| are reasonable, since they should come
- // from |offsetof(Options, some_member)| and |sizeof(Options::some_member)|,
- // respectively.
- return static_cast<const Options*>(buffer)->struct_size >=
- offset + size;
-}
-
-// Macro to invoke |HasOptionsStructMember()| parametrized by member name
-// instead of offset and size.
-//
-// (We can't just give |HasOptionsStructMember()| a member pointer template
-// argument instead, since there's no good/strictly-correct way to get an offset
-// from that.)
-//
-// TODO(vtl): With C++11, use |sizeof(Options::member)| instead.
-#define HAS_OPTIONS_STRUCT_MEMBER(Options, member, buffer) \
- (HasOptionsStructMember< \
- Options, \
- offsetof(Options, member), \
- sizeof(static_cast<const Options*>(buffer)->member)>(buffer))
-
-// Checks that the (standard) |flags| member consists of only known flags. This
-// should only be called if |HAS_OPTIONS_STRUCT_MEMBER()| returned true for the
-// |flags| field.
-//
-// The rationale for *not* ignoring these flags is that the caller should have a
-// way of specifying that certain options not be ignored. E.g., one may have a
-// |MOJO_..._OPTIONS_FLAG_DONT_IGNORE_FOO| flag and a |foo| member; if the flag
-// is set, it will guarantee that the version of the system knows about the
-// |foo| member (and won't ignore it).
-template <class Options>
-bool AreOptionsFlagsAllKnown(const void* buffer, uint32_t known_flags) {
- return (static_cast<const Options*>(buffer)->flags & ~known_flags) == 0;
-}
-
-// Does basic cursory checks on |in_options| (|struct_size| and |flags|; |flags|
-// must immediately follow |struct_size|); |in_options| must be non-null. The
-// following should be done before calling this:
-// - Set |out_options| to the default options.
-// - If |in_options| is null, don't continue (success).
-// This function then:
-// - Checks if (according to |IsOptionsStructPointerAndSizeValid()|),
-// |struct_size| is valid; if not returns |MOJO_RESULT_INVALID_ARGUMENT|.
-// - If |in_options| has a |flags| field, checks that it only has
-// |known_flags| set; if so copies it to |out_options->flags|, and if not
-// returns |MOJO_RESULT_UNIMPLEMENTED|.
-// - At this point, returns |MOJO_RESULT_OK|.
template <class Options>
-MojoResult ValidateOptionsStructPointerSizeAndFlags(
- const Options* in_options,
- uint32_t known_flags,
- Options* out_options) {
- COMPILE_ASSERT(offsetof(Options, flags) == sizeof(uint32_t),
- Options_flags_doesnt_immediately_follow_struct_size);
-
- if (!IsOptionsStructPointerAndSizeValid<Options>(in_options))
- return MOJO_RESULT_INVALID_ARGUMENT;
-
- if (HAS_OPTIONS_STRUCT_MEMBER(Options, flags, in_options)) {
- if (!AreOptionsFlagsAllKnown<Options>(in_options, known_flags))
- return MOJO_RESULT_UNIMPLEMENTED;
- out_options->flags = in_options->flags;
+class UserOptionsReader {
+ public:
+ // Constructor from a |UserPointer<const Options>| (which it checks -- this
+ // constructor has side effects!).
+ // Note: We initialize |options_reader_| without checking, since we do a check
+ // in |GetSizeForReader()|.
+ explicit UserOptionsReader(UserPointer<const Options> options)
+ : options_reader_(UserPointer<const char>::Reader::NoCheck(),
+ options.template ReinterpretCast<const char>(),
+ GetSizeForReader(options)) {
+ COMPILE_ASSERT(offsetof(Options, struct_size) == 0,
+ Options_struct_size_not_first_member);
+ // TODO(vtl): With C++11, compile-assert that |sizeof(Options::struct_size)
+ // == sizeof(uint32_t)| somewhere.
}
- return MOJO_RESULT_OK;
-}
+ bool is_valid() const {
+ return !!options_reader_.GetPointer();
+ }
+
+ const Options& options() const {
+ DCHECK(is_valid());
+ return *reinterpret_cast<const Options*>(options_reader_.GetPointer());
+ }
+
+ // Checks that the given (variable-size) |options| passed to the constructor
+ // (plausibly) has a member at the given offset with the given size. You
+ // probably want to use |OPTIONS_STRUCT_HAS_MEMBER()| instead.
+ bool HasMember(size_t offset, size_t size) const {
+ DCHECK(is_valid());
+ // We assume that |offset| and |size| are reasonable, since they should come
+ // from |offsetof(Options, some_member)| and |sizeof(Options::some_member)|,
+ // respectively.
+ return options().struct_size >= offset + size;
+ }
+
+ private:
+ static inline size_t GetSizeForReader(UserPointer<const Options> options) {
+ uint32_t struct_size =
+ options.template ReinterpretCast<const uint32_t>().Get();
+ if (struct_size < sizeof(uint32_t))
+ return 0;
+
+ // Check the full requested size.
+ // Note: Use |MOJO_ALIGNOF()| here to match the exact macro used in the
+ // declaration of Options structs.
+ internal::CheckUserPointerWithSize<MOJO_ALIGNOF(Options)>(options.pointer_,
+ struct_size);
+ options.template ReinterpretCast<const char>().CheckArray(struct_size);
+ // But we'll never look at more than |sizeof(Options)| bytes.
+ return std::min(static_cast<size_t>(struct_size), sizeof(Options));
+ }
+
+ UserPointer<const char>::Reader options_reader_;
+
+ DISALLOW_COPY_AND_ASSIGN(UserOptionsReader);
+};
+
+// Macro to invoke |UserOptionsReader<Options>::HasMember()| parametrized by
+// member name instead of offset and size.
+//
+// (We can't just give |HasMember()| a member pointer template argument instead,
+// since there's no good/strictly-correct way to get an offset from that.)
+//
+// TODO(vtl): With C++11, use |sizeof(Options::member)| instead of (the
+// contortion below). We might also be able to pull out the type |Options| from
+// |reader| (using |decltype|) instead of requiring a parameter.
+#define OPTIONS_STRUCT_HAS_MEMBER(Options, member, reader) \
+ reader.HasMember(offsetof(Options, member), sizeof(reader.options().member))
} // namespace system
} // namespace mojo
« no previous file with comments | « mojo/system/message_pipe_dispatcher.cc ('k') | mojo/system/options_validation_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698