A cockroach-sized code review (xdong 2010-01-02 16:50:56)
idlec...@gmail.com
I'd like you to do a code review. Please review the following patch:
commit 4dda2829e09c2b30dbb3289b33f9918f5d83ff91
Author: Dong Xiaohu <xdong@xdong-desktop.(none)>
Date: Sat Jan 2 16:44:44 2010 +0800
1. Fix warning of unused parameters in release build
2. Fix warning complained by g++ 4.4.1 in Ubuntu 9.10. Detailed information can be found at: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41874
diff --git a/extensions/qt_script_runtime/js_script_context.cc b/extensions/qt_script_runtime/js_script_context.cc
index 217af1e..cad786f 100755
--- a/extensions/qt_script_runtime/js_script_context.cc
+++ b/extensions/qt_script_runtime/js_script_context.cc
@@ -535,6 +535,8 @@ bool ResolverScriptClass::supportsExtension(Extension extension) const {
QVariant ResolverScriptClass::extension(Extension extension,
const QVariant &argument) {
+
+ GGL_UNUSED(extension);
ASSERT(call_slot_ && extension == Callable);
DLOG("Object called as function");
QScriptContext *context = qvariant_cast<QScriptContext*>(argument);
diff --git a/extensions/smjs_script_runtime/CMakeLists.txt b/extensions/smjs_script_runtime/CMakeLists.txt
index 9c955ce..d641011 100644
--- a/extensions/smjs_script_runtime/CMakeLists.txt
+++ b/extensions/smjs_script_runtime/CMakeLists.txt
@@ -23,6 +23,7 @@ ADD_SUBDIRECTORY(tests)
# Added -fcheck-new to disable the warning about operator new returning NULL
# in jscntxt.h.
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fcheck-new")
+STRING(REPLACE "-Wextra" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
SET(SRCS
converter.cc
diff --git a/ggadget/content_item.cc b/ggadget/content_item.cc
index e719863..7150e90 100644
--- a/ggadget/content_item.cc
+++ b/ggadget/content_item.cc
@@ -278,6 +278,7 @@ void ContentItem::AttachContentArea(ContentAreaElement *content_area) {
}
void ContentItem::DetachContentArea(ContentAreaElement *content_area) {
+ GGL_UNUSED(content_area);
ASSERT(impl_->content_area_ == content_area);
impl_->content_area_ = NULL;
Unref();
diff --git a/ggadget/dbus/dbus_proxy.cc b/ggadget/dbus/dbus_proxy.cc
index 12e6271..65a7e71 100644
--- a/ggadget/dbus/dbus_proxy.cc
+++ b/ggadget/dbus/dbus_proxy.cc
@@ -509,6 +509,7 @@ class DBusProxy::Impl : public SmallObject<> {
static DBusHandlerResult BusFilter(DBusConnection *bus,
DBusMessage *message,
void *user_data) {
+ GGL_UNUSED(bus);
if (dbus_message_get_type(message) != DBUS_MESSAGE_TYPE_SIGNAL)
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
diff --git a/ggadget/dbus/dbus_utils.cc b/ggadget/dbus/dbus_utils.cc
index 16334f5..2dd2861 100644
--- a/ggadget/dbus/dbus_utils.cc
+++ b/ggadget/dbus/dbus_utils.cc
@@ -1436,6 +1436,7 @@ class DBusMainLoopClosure::Impl {
SetEnabled(dbus_watch_get_enabled(watch));
}
virtual bool Call(MainLoopInterface *main_loop, int watch_id) {
+ GGL_UNUSED(main_loop);
ASSERT(impl_);
ASSERT(main_loop == impl_->main_loop_);
ASSERT(watch_id == read_id_ || watch_id == write_id_);
@@ -1466,6 +1467,7 @@ class DBusMainLoopClosure::Impl {
return true;
}
virtual void OnRemove(MainLoopInterface* main_loop, int watch_id) {
+ GGL_UNUSED(main_loop);
ASSERT(main_loop == impl_->main_loop_);
ASSERT(watch_id == read_id_ || watch_id == write_id_);
if (read_id_ == watch_id)
@@ -1548,6 +1550,8 @@ class DBusMainLoopClosure::Impl {
SetEnabled(dbus_timeout_get_enabled(timeout));
}
virtual bool Call(MainLoopInterface *main_loop, int watch_id) {
+ GGL_UNUSED(main_loop);
+ GGL_UNUSED(watch_id);
ASSERT(impl_);
ASSERT(main_loop == impl_->main_loop_);
ASSERT(watch_id == watch_id_);
@@ -1575,6 +1579,8 @@ class DBusMainLoopClosure::Impl {
return true;
}
virtual void OnRemove(MainLoopInterface* main_loop, int watch_id) {
+ GGL_UNUSED(main_loop);
+ GGL_UNUSED(watch_id);
ASSERT(main_loop == impl_->main_loop_);
ASSERT(watch_id == watch_id_);
watch_id_ = -1;
diff --git a/ggadget/elements.cc b/ggadget/elements.cc
index c32c964..1b67a3c 100644
--- a/ggadget/elements.cc
+++ b/ggadget/elements.cc
@@ -59,7 +59,7 @@ class Elements::Impl : public SmallObject<> {
do { for (size_t i_i = 0; i_i < children_.size(); i_i++) \
ASSERT_ELEMENT_INDEX(children_[i_i], i_i); } while (false)
#else
-#define ASSERT_ELEMENTS_INTEGRITY do; while (false)
+#define ASSERT_ELEMENTS_INTEGRITY do{} while (false)
#endif
size_t GetCount() {
diff --git a/ggadget/logger.cc b/ggadget/logger.cc
index 53cee0b..a854b6f 100644
--- a/ggadget/logger.cc
+++ b/ggadget/logger.cc
@@ -130,6 +130,7 @@ void PushLogContext(void *context) {
}
void PopLogContext(void *log_context) {
+ GGL_UNUSED(log_context);
if (!g_log_destroyed) {
ASSERT(log_context == g_log.context_stack.back());
g_log.context_stack.pop_back();
diff --git a/ggadget/scriptable_helper.cc b/ggadget/scriptable_helper.cc
index 5425222..e68d4e9 100644
--- a/ggadget/scriptable_helper.cc
+++ b/ggadget/scriptable_helper.cc
@@ -482,6 +482,7 @@ class ClassSignalSetter : public Slot1<void, Slot *> {
}
virtual ResultVariant Call(ScriptableInterface *obj,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
ASSERT(argc == 1);
Signal *signal = class_signal_->GetSignal(obj);
Slot *slot = VariantValue<Slot *>()(argv[0]);
diff --git a/ggadget/slot.h b/ggadget/slot.h
index add3a15..a21c372 100644
--- a/ggadget/slot.h
+++ b/ggadget/slot.h
@@ -163,6 +163,7 @@ class FunctorSlot0 : public Slot0<R> {
FunctorSlot0(F functor) : functor_(functor) { }
virtual ResultVariant Call(ScriptableInterface *,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
return ResultVariant(Variant(functor_()));
@@ -186,6 +187,7 @@ class FunctorSlot0<void, F> : public Slot0<void> {
FunctorSlot0(F functor) : functor_(functor) { }
virtual ResultVariant Call(ScriptableInterface *,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
functor_();
@@ -213,6 +215,7 @@ class MethodSlot0 : public Slot0<R> {
int argc, const Variant argv[]) const {
// object parameter is ignored because object is bound when this object
// is constructed.
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
return ResultVariant(Variant((object_->*method_)()));
@@ -240,6 +243,7 @@ class MethodSlot0<void, T, M> : public Slot0<void> {
// object parameter is ignored because object is bound when this object
// is constructed.
GGL_UNUSED(object);
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
(object_->*method_)();
@@ -275,6 +279,7 @@ class UnboundMethodSlot0 : public Slot0<R> {
}
virtual ResultVariant Call(ScriptableInterface *object,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
ASSERT(object);
@@ -303,6 +308,7 @@ class UnboundMethodSlot0<void, T, M> : public Slot0<void> {
void operator()(T *object) const { Call(object, 0, NULL); }
virtual ResultVariant Call(ScriptableInterface *object,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
ASSERT(object);
@@ -339,6 +345,7 @@ class DelegatedMethodSlot0 : public Slot0<R> {
}
virtual ResultVariant Call(ScriptableInterface *object,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
ASSERT(object && delegate_getter_(down_cast<T *>(object)));
@@ -371,6 +378,7 @@ class DelegatedMethodSlot0<void, T, M, DelegateGetter> : public Slot0<void> {
void operator()(T *object) const { Call(object, 0, NULL); }
virtual ResultVariant Call(ScriptableInterface *object,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
ASSERT(object && delegate_getter_(down_cast<T *>(object)));
@@ -435,6 +443,7 @@ class FunctorSlotClosure0 : public Slot0<R> {
FunctorSlotClosure0(F functor, PA pa) : functor_(functor), pa_(pa) { }
virtual ResultVariant Call(ScriptableInterface *,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
return ResultVariant(Variant(functor_(pa_)));
@@ -459,6 +468,7 @@ class FunctorSlotClosure0<void, F, PA> : public Slot0<void> {
FunctorSlotClosure0(F functor, PA pa) : functor_(functor), pa_(pa) { }
virtual ResultVariant Call(ScriptableInterface *,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
functor_(pa_);
@@ -486,6 +496,8 @@ class MethodSlotClosure0 : public Slot0<R> {
: obj_(obj), method_(method), pa_(pa) { }
virtual ResultVariant Call(ScriptableInterface *,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
+ GGL_UNUSED(argv);
ASSERT(argc == 0);
return ResultVariant(Variant((obj_->*method_)(pa_)));
}
@@ -511,6 +523,7 @@ class MethodSlotClosure0<void, T, M, PA> : public Slot0<void> {
: obj_(obj), method_(method), pa_(pa) { }
virtual ResultVariant Call(ScriptableInterface *,
int argc, const Variant argv[]) const {
+ GGL_UNUSED(argc);
GGL_UNUSED(argv);
ASSERT(argc == 0);
(obj_->*method_)(pa_);
@@ -727,6 +740,7 @@ class FunctorSlot##n : public Slot##n<R, _arg_type_names> { \
FunctorSlot##n(F functor) : functor_(functor) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
return ResultVariant(Variant(functor_(_call_args))); \
@@ -748,6 +762,7 @@ class FunctorSlot##n<void, _arg_type_names, F> : \
FunctorSlot##n(F functor) : functor_(functor) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
functor_(_call_args); \
@@ -769,6 +784,7 @@ class MethodSlot##n : public Slot##n<R, _arg_type_names> { \
MethodSlot##n(T *obj, M method) : obj_(obj), method_(method) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
return ResultVariant(Variant((obj_->*method_)(_call_args))); \
@@ -791,6 +807,7 @@ class MethodSlot##n<void, _arg_type_names, T, M> : \
MethodSlot##n(T *obj, M method) : obj_(obj), method_(method) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
(obj_->*method_)(_call_args); \
@@ -823,6 +840,7 @@ class UnboundMethodSlot##n : public Slot##n<R, _arg_type_names> { \
} \
virtual ResultVariant Call(ScriptableInterface *obj, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
ASSERT(obj); \
@@ -854,6 +872,7 @@ class UnboundMethodSlot##n<void, _arg_type_names, T, M> : \
} \
virtual ResultVariant Call(ScriptableInterface *obj, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
ASSERT(obj); \
@@ -890,6 +909,7 @@ class DelegatedMethodSlot##n : public Slot##n<R, _arg_type_names> { \
} \
virtual ResultVariant Call(ScriptableInterface *obj, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
ASSERT(obj && delegate_getter_(down_cast<T *>(obj))); \
@@ -926,6 +946,7 @@ class DelegatedMethodSlot##n<void, _arg_type_names, T, M, DelegateGetter> \
} \
virtual ResultVariant Call(ScriptableInterface *obj, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
ASSERT(obj && delegate_getter_(down_cast<T *>(obj))); \
@@ -971,6 +992,7 @@ class FunctorSlotClosure##n : public Slot##n<R, _arg_type_names> { \
FunctorSlotClosure##n(F functor, PA pa) : functor_(functor), pa_(pa) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
return ResultVariant(Variant(functor_(_call_args, pa_))); \
@@ -993,6 +1015,7 @@ class FunctorSlotClosure##n<void, _arg_type_names, F, PA> : \
FunctorSlotClosure##n(F functor, PA pa) : functor_(functor), pa_(pa) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
functor_(_call_args, pa_); \
@@ -1016,6 +1039,7 @@ class MethodSlotClosure##n : public Slot##n<R, _arg_type_names> { \
: obj_(obj), method_(method), pa_(pa) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
return ResultVariant(Variant((obj_->*method_)(_call_args, pa_))); \
@@ -1040,6 +1064,7 @@ class MethodSlotClosure##n<void, _arg_type_names, T, M, PA> : \
: obj_(obj), method_(method), pa_(pa) { } \
virtual ResultVariant Call(ScriptableInterface *, \
int argc, const Variant argv[]) const { \
+ GGL_UNUSED(argc); \
GGL_UNUSED(argv); \
ASSERT(argc == n); \
(obj_->*method_)(_call_args, pa_); \
diff --git a/ggadget/variant.cc b/ggadget/variant.cc
index 3360794..ba6a6b0 100644
--- a/ggadget/variant.cc
+++ b/ggadget/variant.cc
@@ -65,10 +65,12 @@ Variant::~Variant() {
// This typedef is required because ...~std::string() may cause error.
typedef std::string String;
// Don't delete because the pointer is allocated in place.
- reinterpret_cast<String *>(&v_.string_place_)->~String();
+ void *data = v_.string_place_;
+ reinterpret_cast<String *>(data)->~String();
} else if (type_ == TYPE_UTF16STRING) {
// Don't delete because the pointer is allocated in place.
- reinterpret_cast<UTF16String *>(&v_.utf16_string_place_)->~UTF16String();
+ void *data = v_.utf16_string_place_;
+ reinterpret_cast<UTF16String *>(data)->~UTF16String();
}
}
@@ -80,13 +82,16 @@ Variant &Variant::operator=(const Variant &source) {
// This typedef is required because ...~std::string() may cause error.
typedef std::string String;
// Don't delete because the pointer is allocated in place.
- reinterpret_cast<String *>(&v_.string_place_)->~String();
+ void *data = v_.string_place_;
+ reinterpret_cast<String *>(data)->~String();
} else if (type_ == TYPE_UTF16STRING) {
// Don't delete because the pointer is allocated in place.
- reinterpret_cast<UTF16String *>(&v_.utf16_string_place_)->~UTF16String();
+ void *data = v_.utf16_string_place_;
+ reinterpret_cast<UTF16String *>(data)->~UTF16String();
}
type_ = source.type_;
+ const void *data = source.v_.string_place_;
switch (type_) {
case TYPE_VOID:
break;
@@ -102,11 +107,11 @@ Variant &Variant::operator=(const Variant &source) {
case TYPE_STRING:
case TYPE_JSON:
new (&v_.string_place_) std::string(
- *reinterpret_cast<const std::string *>(&source.v_.string_place_));
+ *reinterpret_cast<const std::string *>(data));
break;
case TYPE_UTF16STRING:
new (&v_.utf16_string_place_) UTF16String(
- *reinterpret_cast<const UTF16String *>(&source.v_.utf16_string_place_));
+ *reinterpret_cast<const UTF16String *>(data));
break;
case TYPE_SCRIPTABLE:
v_.scriptable_value_ = source.v_.scriptable_value_;
@@ -187,6 +192,7 @@ static std::string FitString(const std::string &input) {
// Used in unittests.
std::string Variant::Print() const {
+ const void *data = v_.string_place_;
switch (type_) {
case TYPE_VOID:
return "VOID";
@@ -199,7 +205,7 @@ std::string Variant::Print() const {
case TYPE_STRING:
return std::string("STRING:") +
// Print "(nil)" for NULL string pointer.
- FitString(*reinterpret_cast<const std::string *>(&v_.string_place_));
+ FitString(*reinterpret_cast<const std::string *>(data));
case TYPE_JSON:
return std::string("JSON:") +
FitString(VariantValue<JSONString>()(*this).value);
@@ -207,7 +213,7 @@ std::string Variant::Print() const {
std::string utf8_string;
ConvertStringUTF16ToUTF8(
// Print "(nil)" for NULL string pointer.
- *reinterpret_cast<const UTF16String *>(&v_.utf16_string_place_),
+ *reinterpret_cast<const UTF16String *>(data),
&utf8_string);
return "UTF16STRING:" + FitString(utf8_string);
}
diff --git a/ggadget/view.cc b/ggadget/view.cc
index 4b479c5..9c19d37 100644
--- a/ggadget/view.cc
+++ b/ggadget/view.cc
@@ -110,6 +110,7 @@ class View::Impl : public SmallObject<> {
}
virtual bool Call(MainLoopInterface *main_loop, int watch_id) {
+ GGL_UNUSED(watch_id);
ASSERT(event_.GetToken() == watch_id);
ScopedLogContext log_context(impl_->gadget_);
@@ -157,6 +158,7 @@ class View::Impl : public SmallObject<> {
virtual void OnRemove(MainLoopInterface *main_loop, int watch_id) {
GGL_UNUSED(main_loop);
+ GGL_UNUSED(watch_id);
ASSERT(event_.GetToken() == watch_id);
delete this;
}
diff --git a/ggadget/xml_dom.cc b/ggadget/xml_dom.cc
index 0f02d3f..3718067 100644
--- a/ggadget/xml_dom.cc
+++ b/ggadget/xml_dom.cc
@@ -2029,7 +2029,7 @@ class DOMDocumentFragment : public DOMNodeBase<DOMDocumentFragmentInterface> {
// Because DOMDocumentFragment can't be child of any node, the indent
// should always be zero.
ASSERT(indent == 0);
- GetImpl()->AppendChildrenXML(0, xml);
+ GetImpl()->AppendChildrenXML(indent, xml);
}
protected:
@@ -2337,7 +2337,7 @@ class DOMDocument : public DOMNodeBase<DOMDocumentInterface> {
virtual void AppendXML(size_t indent, std::string *xml) {
ASSERT(indent == 0);
xml->append(kStandardXMLDecl);
- GetImpl()->AppendChildrenXML(0, xml);
+ GetImpl()->AppendChildrenXML(indent, xml);
}
virtual XMLParserInterface *GetXMLParser() const {
This is a semiautomated message from "gitmail". Complaints or suggestions?
Mail edy...@gmail.com.
Xianzhu Wang
2010/1/2 <idlec...@gmail.com>: