commit 70f6625e011a691b502abb68e3453a999e536e51
author Steven Moreland <smoreland@google.com> Thu Jan 09 17:26:57 2020 -0800
committer Steven Moreland <smoreland@google.com> Fri Jan 10 01:43:08 2020 +0000
tree aa337e5ae830fb99337e13eec417b66c4a71e59e
parent 8c4e645b16003facefa5f31196bf3cfd247191c6

Remove ErrorMode_Ignore.

Before:

    _hidl_err = ... writeInt ...
    _hidl_err = ... writeLong ...
    ...
    // end traces
    ...
    _hidl_cb(...)
    return _hidl_err

This has two problems:
- _hidl_cb should not be called if there is an error
- _hidl_err might be an error initially and then not an error later
(although this is theoretical. Since HIDL structures are fixed-length on
the parcel itself, I can't find any case where this be a problem).

After:
    _hidl_err = ... writeInt ...
    if (_hidl_err != OK) goto _hidl_error;
    _hidl_err = ... writeLong ...
    if (_hidl_err != OK) goto _hidl_error;
    ...
  _hidl_error:
    // end traces
    ...
    if (_hidl_err != OK) return _hidl_err;
    _hidl_cb(...)
    return _hidl_err

Fixes: 129708508
Test: ./test/run_all_device_tests.sh
Change-Id: I833a6b979871ba53ccbdfdb7ae5eef8403ce0772

PointerType.cpp

diff --git a/PointerType.cpp b/PointerType.cpp
index f6eaf54..29b25f1 100644
--- a/PointerType.cpp
+++ b/PointerType.cpp
@@ -50,9 +50,12 @@
         const std::string& /*parcelObj*/,
         bool /*parcelObjIsPointer*/,
         bool /*isReader*/,
-        ErrorMode /*mode*/) const {
+        ErrorMode mode) const {
     out << "(void)" << name << ";\n";
     out << "LOG_ALWAYS_FATAL(\"Pointer is only supported in passthrough mode\");\n\n";
+
+    // always use label if mode is goto
+    handleError(out, mode);
 }
 
 void PointerType::emitReaderWriterEmbedded(

Type.cpp

diff --git a/Type.cpp b/Type.cpp
index 5873cc0..9eab914 100644
--- a/Type.cpp
+++ b/Type.cpp
@@ -507,12 +507,6 @@
 
 void Type::handleError(Formatter &out, ErrorMode mode) {
     switch (mode) {
-        case ErrorMode_Ignore:
-        {
-            out << "/* _hidl_err ignored! */\n\n";
-            break;
-        }
-
         case ErrorMode_Goto:
         {
             out << "if (_hidl_err != ::android::OK) { goto _hidl_error; }\n\n";

Type.h

diff --git a/Type.h b/Type.h
index 6ddd11d..390ffda 100644
--- a/Type.h
+++ b/Type.h
@@ -194,7 +194,6 @@
     virtual std::string getVtsValueName() const;
 
     enum ErrorMode {
-        ErrorMode_Ignore,
         ErrorMode_Goto,
         ErrorMode_Break,
         ErrorMode_Return,

generateCpp.cpp

diff --git a/generateCpp.cpp b/generateCpp.cpp
index 510a96e..72c8e82 100644
--- a/generateCpp.cpp
+++ b/generateCpp.cpp
@@ -1515,7 +1515,11 @@
                 "_hidl_reply",
                 true, /* parcelObjIsPointer */
                 false, /* isReader */
-                Type::ErrorMode_Ignore);
+                Type::ErrorMode_Goto);
+
+        out.unindent();
+        out << "_hidl_error:\n";
+        out.indent();
 
         generateCppInstrumentationCall(
                 out,
@@ -1523,6 +1527,7 @@
                 method,
             superInterface);
 
+        out << "if (_hidl_err != ::android::OK) { return _hidl_err; }\n";
         out << "_hidl_cb(*_hidl_reply);\n";
     } else {
         if (returnsValue) {
@@ -1572,16 +1577,23 @@
                         true /* parcelObjIsPointer */,
                         arg,
                         false /* reader */,
-                        Type::ErrorMode_Ignore,
+                        Type::ErrorMode_Goto,
                         true /* addPrefixToName */);
             }
 
+            if (!method->results().empty()) {
+                out.unindent();
+                out << "_hidl_error:\n";
+                out.indent();
+            }
+
             generateCppInstrumentationCall(
                     out,
                     InstrumentationEvent::SERVER_API_EXIT,
                     method,
                     superInterface);
 
+            out << "if (_hidl_err != ::android::OK) { return; }\n";
             out << "_hidl_cb(*_hidl_reply);\n";
 
             out.unindent();