trafodion-codereview mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zellerh <...@git.apache.org>
Subject [GitHub] trafodion pull request #1504: [TRAFODION-3009] Streamline error handling in ...
Date Tue, 03 Apr 2018 17:54:55 GMT
Github user zellerh commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1504#discussion_r178901892
  
    --- Diff: core/sql/executor/ExExeUtilCli.cpp ---
    @@ -2091,27 +2084,57 @@ Lng32 ExeCliInterface::deleteContext(char* contextHandle) // in
buf contains con
     Lng32 ExeCliInterface::retrieveSQLDiagnostics(ComDiagsArea *toDiags)
     {
       Lng32 retcode;
    -
    +  ex_assert(toDiags != NULL, "ComDiagsArea is null");
       if (diagsArea_ != NULL)
         {
           diagsArea_->clear();
           diagsArea_->deAllocate();
         }
    +  retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
    +  SQL_EXEC_ClearDiagnostics(NULL);
    +  return retcode;
    +}
     
    -  if (toDiags != NULL)
    -    {
    -      retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
    -      SQL_EXEC_ClearDiagnostics(NULL);
    -    }
    -  else
    +
    +ComDiagsArea *ExeCliInterface::allocAndRetrieveSQLDiagnostics(ComDiagsArea *&toDiags)
    +{
    +  Lng32 retcode;
    +
    +  if (diagsArea_ != NULL)
         {
    -      diagsArea_ = ComDiagsArea::allocate(heap_);
    -      retcode = SQL_EXEC_MergeDiagnostics_Internal(*diagsArea_);
    +      diagsArea_->clear();
    +      diagsArea_->deAllocate();
         }
    +  if (toDiags == NULL)
    +      toDiags = ComDiagsArea::allocate(heap_);
     
    -  return retcode;
    +  retcode = SQL_EXEC_MergeDiagnostics_Internal(*toDiags);
    +  SQL_EXEC_ClearDiagnostics(NULL);
    + 
    +  if (retcode == 0)
    +     return toDiags;
    +  else
    +     return NULL;
    --- End diff --
    
    See also below for a possible leak. Instead of returning NULL, it may be better to add
a condition to the allocated diags area. That would make it easier for the caller to check
(no check needed in most cases).


---

Mime
View raw message