[GitHub] apeforest commented on a change in pull request #12076: Add GetName function in Symbol class for cpp package

[GitBox <gi...@apache.org>]

apeforest commented on a change in pull request #12076: Add GetName function in Symbol class for cpp package
URL: https://github.com/apache/incubator-mxnet/pull/12076#discussion_r208806119
 
 

 ##########
 File path: cpp-package/include/mxnet-cpp/symbol.hpp
 ##########
 @@ -172,6 +172,13 @@ inline std::vector<std::string> Symbol::ListAuxiliaryStates() const {
   return ret;
 }
 
+inline std::string Symbol::GetName() const {
+  int success;
+  const char* out_name;
+  CHECK_EQ(MXSymbolGetName(GetHandle(), &out_name, &success), 0);
 
 Review comment:
   What if success == 0? Based on the function description, it seems that out_name could be NULL even if the returned value is 0:
   
   ```
   /*!
    * \brief Get string attribute from symbol
    * \param symbol the source symbol
    * \param key The key of the symbol.
    * \param out The result attribute, can be NULL if the attribute do not exist.
    * \param success Whether the result is contained in out.
    * \return 0 when success, -1 when failure happens
    */
   int NNSymbolGetAttr(SymbolHandle in,
                                const char* key,
                                const char** out,
                                int *out);
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services