1. 28 4月, 2020 1 次提交
    • P
      Let Fts tolerate the in-progress 'starting up' case on primary nodes. · 5222ad86
      Paul Guo 提交于
      commit d453a4aa implemented that for the crash
      recovery case (not marking the node down and then not promoting the mirror). It
      seems that we should do that for the usual "starting up" case also(i.e.
      CAC_STARTUP), besides for the existing "in recovery mode" case (i.e.
      CAC_RECOVERY).
      
      We've seen that fts promotes the "starting up" primary during isolation2
      testing due to 'pg_ctl restart'. In this patch we check recovery progress for
      both CAC_STARTUP an CAC_RECOVERY during fts probe and thus can avoid this.
      Reviewed-by: NAshwin Agrawal <aagrawal@pivotal.io>
      
      cherry-picked from d71b3afd
      
      On master the commit message was eliminated by mistake. Added back on gpdb6.
      5222ad86
  2. 21 3月, 2020 1 次提交
    • D
      Fix fts probe response deserialization (#9770) · 60259507
      David Kimura 提交于
      In probeRecordResponse(), columns are read using PQgetvalue() and cast to int
      pointers. Then values are stored as bools.  However, SendFtsResponse() writes
      only one byte per column so we really probably should not be casting to int
      pointers as we have no guarantee what the high bits will be. This could lead to
      unexpected behavior.
      
      Issue is that depending on the underlying type of bool, we could
      inadvertantly use the undefined high bits behind the int pointer cast
      dereference to decide the value to store into the bool. Thus we could
      read unexpected values for isMirrorAlive, isInSync, isSyncRepEnabled,
      etc.
      
      For example, in this code `b` and `c` have different values:
      ```
      int i = 0x1000;
      unsigned char c = *&i;
      bool b = *&i;
      ```
      
      This was caught during postgres 12 merge iteration due to postgres 11 commit
      9a95a77d which changed the preferred definition of bool to use stdbool.h
      instead of typedef unsigned char.
      
      Issue can be reproduced outside the merge branch by simply adding includes for
      `<stdbool.h>` inside fts.c and ftsprobe.h files.
      Co-authored-by: NAshwin Agrawal <aagrawal@pivotal.io>
      (cherry picked from commit fe4bcafa)
      60259507
  3. 25 10月, 2019 1 次提交
    • H
      Enable -Wimplicit-fallthrough by default(#8809) · 094a2d76
      Hao Wu 提交于
      GCC 7+ has a compiling option, -Wimplicit-fallthrough, which will
      generate a warning/error if the code falls through cases(or default)
      implicitly. The implicity may cause some bugs that are hardly to catch.
      
      To enable this compile option, all switch-cases are not allowed to
      fall through implicitly. To accomplish this target, 2 steps are done:
      1. Append a comment line /* fallthrough */ or like at the end of case block.
      2. Add break clause at the end of case block, if the last statement is
        ereport(ERROR) or like.
      
      When -Wimplicit-fallthrough is enabled, -Werror=implicit-fallthrough
      is also set. So fallthrough must be done explicitly.
      Re-generate configure after modifying configure.in
      
      No implicit-fallthrough for gpmapreduce and orafce
      
      * Change CFLAGS to CXXFLAGS when using CXX to link postgres
      094a2d76
  4. 07 8月, 2019 1 次提交
    • K
      FTS: Ensure fresh results from a manual probe request · 0b73bd6c
      Kalen Krempely 提交于
      This patch addresses two main scenarios:
      1) Allowing multiple probes both internal and external to reuse the same
      results when appropriate (ie: piggybacking on previous results). Multiple
      requests should share the same results if they all request before the start of
      a new fts loop, and after the results of the previous probe.
      
      2) Ensuring fresh results from an external probe. When a request occurs during
      a current probe in progress, this request should get fresh results rather
      "piggybacking" or using the current results.
      
      We use similar logic as the checkpointer code to detect whether a probe is in
      progress with a probe start tick and probe end tick. To request a probe, we
      send a signal requesting a fts results, then wait for a new loop to start,
      then wait again for that current loop to finish. This implementation uses a
      busy wait loop, which includes a short sleep. In the future, we can
      leverage the upstream conditaion variable implementation which enables us
      to signal multiple fts notify processes.
      
      This was done via a manual cherry-pick from
      a674b6b3025b9dc56c4cb34b3330f8b7bc1bf757.
      Co-authored-by: NSoumyadeep Chakraborty <sochakraborty@pivotal.io>
      Co-authored-by: NKalen Krempely <kkrempely@pivotal.io>
      Co-authored-by: NDavid Krieger <dkrieger@pivotal.io>
      Co-authored-by: NTaylor Vesely <tvesely@pivotal.io>
      Co-Authored-by: NAlexandra Wang <lewang@pivotal.io>
      Co-Authored-by: NJimmy Yih <jyih@pivotal.io>
      0b73bd6c
  5. 05 7月, 2019 1 次提交
    • P
      Avoid dispatcher accessing catalog outside of transaction · 5f9cf8b9
      Pengzhou Tang 提交于
      In phase 2 of 2PC, if COMMIT_PREPARED or ABORT_PREPARED failed, dispather
      disconnect and destroy all gangs and fetch the latest configurations
      from catalog gp_segment_configuration which is updated by FTS, then do
      RETRY_COMMIT_PREPARED or RETRY_ABORT_PREPARED. The problem is, in phase
      2 we have marked current transaction state to TRANS_COMMIT/ABORT and it
      is not safe to access catalog outside of the transaction. We added a
      hack in RelationIdGetRelation to workaround this, but it does not
      resolve the problem.
      
      To avoid accessing catalog out of the transaction and get the latest
      segment configurations, dispatcher will notify FTS to dump the configs
      from catalog to a flat file and then dispatcher will read the configs
      from that file then. The performance is not taken into consideration
      because retries of COMMIT_PREPARED or ABORT_PREPARED is not that
      caring about it.
      
      This commit also refactor a little bit of CdbComponentDatabaseInfo
      
      * Add a new structure named GpSegConfigEntry which is a copy of entry
        in catalog table gp_segment_configuration.
      * Replace seginfo with GpSegConfigEntry in segadmin.c
      * Add a helper function named dumpGpSegConfigFromCatalog to fetch
        segment configurations from catalog.
      
      Also remove GPDB_94_MERGE_FIXME in RelationIdGetRelation
      
      Co-authored-by Asim R P <apraveen@pivotal.io>
      Reviewed-by: NHeikki Linnakangas <hlinnakangas@pivotal.io>
      Reviewed-by: NDavid Kimura <dkimura@pivotal.io>
      5f9cf8b9
  6. 05 4月, 2019 1 次提交
  7. 23 1月, 2019 1 次提交
    • A
      Validation for gp_dbid and gp_contentid between QD catalog and QE. · 78aed203
      Ashwin Agrawal 提交于
      Since gp_dbid and gp_contentid is stored in conf files on QE, its
      helpful to have validation to compare values between QD catalog table
      gp_segment_configuration and QE. This validation is performed using
      FTS. FTS message includes gp_dbid and gp_contentid values from
      catalog. QE validates the value while handling the FTS message and if
      finds inconsistency PANICS.
      
      This check is mostly targeted during development to catch missed
      handling of gp_dbid and gp_contentid values in config files. For
      future features like pg_upgrade and gpexpand which copy master
      directory and convert it to segment.
      Co-authored-by: NAlexandra Wang <lewang@pivotal.io>
      78aed203
  8. 21 12月, 2018 1 次提交
    • A
      Remove XLogLocationToString*() and guc Debug_print_qd_mirroring. · 6d902a61
      Ashwin Agrawal 提交于
      Most of the code under guc `Debug_print_qd_mirroring` was deleted when special
      code for master mirroring like mmxlog and friends were deleted. Delete the
      remaining pieces and the GUC.
      
      Also, we were carrying diff against upstream with XLogLocationToString*()
      functions. Many of the variants were unused. The ones which were used can be
      easily replaced and avoid need for having static string buffer to construct the
      string and then print.
      6d902a61
  9. 19 12月, 2018 1 次提交
  10. 02 5月, 2018 2 次提交
    • A
      Remove incorrect isRoleMirror assertion for FTS_PROMOTE_SUCCESS. · 874fcdc5
      Ashwin Agrawal 提交于
      Incase promote message is send twice while mirror already undergoing promotion,
      isRoleMirror can be retruned as false. ON mirror in this case promote message is
      ignored. So, on master fts shouldn't check what the isRoleMirror returns.
      874fcdc5
    • A
      FTS detects when primary is in recovery avoiding config change · d453a4aa
      Ashwin Agrawal 提交于
      Previous behavior when primary is in crash recovery FTS probe fails and hence
      qqprimary is marked down. This change provides a recovery progress metric so that
      FTS can detect progress. We added last replayed LSN number inside the error
      message to determine recovery progress. This allows FTS to distinguish between
      recovery in progress and recovery hang or rolling panics. Only when FTS detects
      recovery is not making progress then FTS marks primary down.
      
      For testing a new fault injector is added to allow simulation of recovery hang
      and recovery in progress.
      
      Just fyi...this reverts the reverted commit 7b7219a4.
      Co-authored-by: NAshwin Agrawal <aagrawal@pivotal.io>
      Co-authored-by: NDavid Kimura <dkimura@pivotal.io>
      d453a4aa
  11. 01 5月, 2018 2 次提交
  12. 29 3月, 2018 2 次提交
  13. 20 3月, 2018 2 次提交
    • A
      FtsNotifyProber() shouldn't wait infinitely for FTS probes. · edc3914d
      Asim R P 提交于
      - Use standard PMSignal mechanism to trigger FTS probes from
        dispatcher.
      
      - Use statusVersion flag only to indicate if a probe cycle resulted in
      configuration update. This was overloaded to also unblock callers
      waiting for FTS probe to be triggered and finished.
      
      - Use a separate flag "probeTick" for this purpose.
      Co-authored-by: NAshwin Agrawal <aagrawal@pivotal.io>
      edc3914d
    • A
      Skip marking mirror down by FTS if retryRequested. · 90d1501b
      Ashwin Agrawal 提交于
      Mirror is provided with grace period before being marked as down. FTS probes
      during this grace period interval, return retryRequested to probe request. Based
      on `retryRequested` should retry probes to primary and incase number of retries
      reach retry limit of `gp_fts_probe_retries` must skip marking such mirror down
      as still primary has not given go ahead to mark it down. Only when
      `retryRequested=false`, mirror must be marked down.
      90d1501b
  14. 10 3月, 2018 2 次提交
    • A
      Move probe related functions to ftsprobe.c · 1e8fd668
      Asim R P 提交于
      Also incorporate the following review comments to 5a7aaf43:
      
       * Break the processing of response and updating of cluster
         configuration into two separate functions.
       * Probe timeout is considered as a failure and retried.
       * Added a new state to distinguish whether a response is processed or
         not.
       * Reduce logging volumn by making use of gp_log_fts GUC.
       * And a bunch of renames.
      Co-authored-by: NDavid Kimura <dkimura@pivotal.io>
      1e8fd668
    • A
      Make FTS single process, using async libpq API, remove threads · 5a7aaf43
      Asim R P 提交于
      A couple of problems with the threaded implementation were:
      
      1. The implementation was divided into two steps - spin up threads and
      send PROBE message to all primary segments in the first step.  Only
      after receiving response from all segments, in the second  step, spin up
      threads and send subsequent messages (e.g. syncrep off, promote), if
      necessary.  A single stubborn segment that wouldn't respond in the first
      step would delay the second step for everyone else.
      
      2. Standard elog.c interface cannot be used for error handling as it is
      not thread safe.  As a result, any assertion failures within threads
      could be missed out or cause wierd behavior such as garbage characters
      in process list entry of fts process.
      
      With the single process implementation, both these problems go away.  We
      introduce interal states for a connection from FTS to a segment (primary
      or mirror).  The states are transitioned until a final state is reached.
      Once all connections have reached final state (success or failure), the
      probe cycle ends.
      Co-authored-by: NDavid Kimura <dkimura@pivotal.io>
      Co-authored-by: NTaylor Vesely <tvesely@pivotal.io>
      5a7aaf43
  15. 13 1月, 2018 5 次提交
    • A
      Fix FTS probe context with correct number of primaries · 8900d60a
      Ashwin Agrawal 提交于
      Originally, we skip probing primary without mirrors, however, we still
      use the num_primary_segments to indicate number of requests. This caused
      inconsistency when processing the response, since, not all the primary
      segments got probe response.
      
      The fix is to only track the actual number of probe requests sent.
      
      Author: Xin Zhang <xzhang@pivotal.io>
      Author: Ashwin Agrawal <aagrawal@pivotal.io>
      8900d60a
    • X
      Primary to mirror failover · 4d9c7897
      Xin Zhang 提交于
      - detect primary goes down
      - flip the role to m/p/d/n and p/m/u/n (role/prefer/status/mode) in
        gp_segment_configuration
      - send promotion message to mirror to promote it
      
      Author: Xin Zhang <xzhang@pivotal.io>
      Author: Jacob Champion <pchampion@pivotal.io>
      Author: Asim R P <apraveen@pivotal.io>
      4d9c7897
    • J
      Fix synchronous_standby_names consistency · 3b80a6e2
      Jacob Champion 提交于
      Revert SyncRepUpdateSyncStandbysDefined to its upstream contract: only
      checkpointer may call it, and it acquires the SyncRepLock on its own.
      If we don't need to ensure that the FTS responder sends back
      syncrep=false immediately, the logic is vastly simplified, and we can
      remove the assertion on the receiver end.
      
      Also note that we don't need to set the GUC in memory directly; the
      SIGHUP will load the new GUC from disk, further simplifying things here.
      
      Author: Jacob Champion <pchampion@pivotal.io>
      Author: Ashwin Agrawal <aagrawal@pivotal.io>
      3b80a6e2
    • A
      Add retries using grace period for declaring mirror down. · cd647b1f
      Ashwin Agrawal 提交于
      If fts detects primary as down, it retries n times before marking it down. But
      mirror gets marked as down if connection to primary has not been made or
      lost. This surfaced as problem mostly during cluster start (gpstart), where
      sequence is to start primary and mirror followed by master. In many instances
      when master probed primary, mirror connection was yet to be made and hence up
      mirror in configuration unnecessarily got marked down, if if just few secs latr
      mirror established connection to primary.
      
      So, to avoid such sitations plus make it little resilient against minor network
      glitches, adding variable to record when initialization or disconnection
      happened. Using the same on fts probe find now can find how long mirror didn't
      showed-up. Only if mirror didn't show-up for allowed period (30 secs) for now
      report it was down, else request fts to retry the probe. This logic doesn't
      affect regular flow also avoids any waiting in utilties for specific states
      after cluster restart.
      cd647b1f
    • H
      Remove --disable-segwalrep option, and the #ifdefs. · db7e4020
      Heikki Linnakangas 提交于
      WAL replication is the name of the game on this branch.
      db7e4020
  16. 21 12月, 2017 1 次提交
    • H
      Use strcmp for string comparison. · a7d11627
      Heikki Linnakangas 提交于
      On my laptop, with gcc -O, these comparisons didn't work as intended,
      and as result, the FTS probe messages were not processed, and
      gp_segment_configuration always claimed the mirrors to be down, even though
      they were running OK.
      a7d11627
  17. 16 12月, 2017 2 次提交
    • J
      FTS turn off syncrep for primary when mirror is detected down · 9fbfec7a
      Jimmy Yih 提交于
      Synchronized replication is on by default for Greenplum. When the
      primary's mirror is detected down, the primary will continue to block
      all commits until the mirror comes back up.
      
      This commit introduces a new FTS message that will be sent to the
      primary if an FTS probe detects a mirror is down and primary is stuck
      in synchronous replication state. The new message will allow the
      primary to turn off synchronous replication by setting the GUC
      synchronous_standby_names to empty string (persisted in
      gp_replication.conf) and setting the WalSndCtl shared-memory syncrep
      state. As a result, backends that may be waiting for the mirror to
      receive a commit will be unblocked.
      
      FTS must note mirror's status as down in configuration before syncrep can be
      turned off by the primary.
      
      Author: Jimmy Yih <jyih@pivotal.io>
      Author: Asim R P <apraveen@pivotal.io>
      Author: Jacob Champion <pchampion@pivotal.io>
      9fbfec7a
    • J
      Rename fts probe variables to be more generic · 9a18c185
      Jimmy Yih 提交于
      Along with rename, fix relative unit tests.
      
      Author: Jimmy Yih <jyih@pivotal.io>
      Author: Asim R P <apraveen@pivotal.io>
      9a18c185
  18. 22 11月, 2017 1 次提交
  19. 08 11月, 2017 3 次提交
  20. 30 10月, 2017 1 次提交
  21. 27 10月, 2017 1 次提交
    • A
      Coverity fixes for FTS. · c3a96e1a
      Abhijit Subramanya 提交于
      CID 178114: Performance inefficiencies  (PASS_BY_VALUE)
      The `ProbeConnectionInfo` parameter was being passed to the
      `probeSegmentHelper()` function by value. This is very inefficient. Fixed it by
      passing the parameter as a reference using a pointer.
      
      CID 178115: Calling "pqGetInt" without checking return value (as is done
      elsewhere 52 out of 57 times).
      The return value of `pqGetInt()` function was not being checked in the
      `processProbeResponse()` function. Fixed it by checking for the return value.
      If EOF is returned, a message is logged and false is returned to the caller.
      Signed-off-by: NTaylor Vesely <tvesely@pivotal.io>
      c3a96e1a
  22. 19 10月, 2017 3 次提交
    • A
      Move FTS filerep specific functions to separate file. · 05346eb2
      Ashwin Agrawal 提交于
      Also, fixes bunch of warnings introduced due to fts walrep work.
      05346eb2
    • A
      Initial FTS code for walrep. · 008252c7
      Ashwin Agrawal 提交于
      The main goal of this commit is to detect and reflect mirror's status up or down
      via primary, on master in catalog.
      
      To achieve the above goal:
      
      - Adding IsMirrorUp() function which walks through WalSndCtl and based on
        walsnds state, reflects mirror up or down. So, now this is the only code
        touching the mirror status in gp_segment_configuration. Both up and mirror
        will get detected via FTS hence utility just needs to start the mirror and FST
        would perform the catalog update.
      
      - Currently added FIXME as cannot acquire LWLock in IsMirrorUp(). This
        restriction will be removed once ftsprobe handler is coded as a lightweigth
        backend process like walsender, instead of processing FTS probes on primary in
        ProcessStartupPacket() directly.
      
      - Adding sepaarte file for FTS probe handler code for walrep instead of
        currently present in postmaster.c
      
      - Basic framework for master using the fts process to probe each primary and
        gets the status back. New response structures are defined to ease future
        development for adding support for detecting sync, not-in-sync.
      
      - Persist the mirror status change to catalog tables. This also does the status
        of the primary. Coded to prevent status change to mirror if the primary is
        offline.
      
      Reference to related mailing list discussion:
      https://groups.google.com/a/greenplum.org/d/msg/gpdb-dev/sRAjNHHTqMA/Uzi-0GR4AQAJSigned-off-by: NXin Zhang <xzhang@pivotal.io>
      Signed-off-by: NTaylor Vesely <tvesely@pivotal.io>
      008252c7
    • A
      Use async libpq API for FTS probing · 4cb2d047
      Asim R P 提交于
      Previously we were using socket()/connect() C API directly to send
      libpq messages in FTS.  Using async libpq API opens up the possibility
      of elimintating threads from FTS in future.  Additionally, it opens up
      further possbilities of leveraging other libpq features such as
      authentication in future.
      Signed-off-by: NTaylor Vesely <tvesely@pivotal.io>
      4cb2d047
  23. 01 9月, 2017 1 次提交
  24. 31 8月, 2017 1 次提交
    • J
      Basic FTS probe response for segment WAL replication · 14fef0d6
      Jimmy Yih 提交于
      Current FTS probe relies on file replication data structures mostly
      found in pmModuleState.  This commit establishes an empty FTS probe
      response packet that primaries using segment WAL replication can fill
      out later.
      
      We also introduce the 'w' fault_strategy (WAL replication) to replace
      the 'f' fault_strategy (file replication).
      
      This commit is carved and modified from a previously closed PR:
      https://github.com/greenplum-db/gpdb/pull/2936
      
      Authors: Abhijit Subramanya and Jimmy Yih
      14fef0d6
  25. 14 4月, 2017 1 次提交
    • S
      Fixing Coverity errors for FTS component. · df5460ea
      Shoaib Lari 提交于
      This commit fixes the following Coverity errors.
      
      ftsfilerep.c: 129923 (for fts.c), 160618, 160637, 160639
      ftsprobe.c: 129898
      primary_mirror_mode.c: 160714 (for ftsprobe.c)
      ftssan.c: 160620, 160636, 160638, 157330
      
      The individual Coverity warnings are described below
      
      ftsfilerep.c
      ============
      CID 129923
      ----------
      The Coverity error is reported in fts.c.  However, the fix is in ftsfilerep.c.
      
      In function probePublishUpdate(), function FtsGetPairStateFilerep() will return
      FILEREP_SENTINEL if the segements are in inconsistent state.  This value is
      passed to transition() as the stateOld parameter.  The function transition()
      calls FtsTransitionFilerep() with this value of stateOld.
      
      FtsTransitionFilerep() uses stateOld to index the state_machine_filerep array.
      However, this causes the following warning from Coverity.
      
      Out-of-bounds access (OVERRUN)
      overrun-call: Overrunning callee's array of size 3 by passing argument stateOld
      (which evaluates to 4) in call to transition.
      
      There is an Assert in FtsTransitionFilerep() to check the validity of stateOld.
      However, production code is compiled without Assert, so this is a possible error
      condition at runtime.
      
      The fix is to log an error when an invalid stateOld value is passed to
      FtsTranstionFilerep() and return without indexing the array.
      
      CID 160618
      ----------
      Added missing break in FtsResolveStateFilerep() for FILEREP_PUS_MUS and
      FILEREP_PUR_MUR cases.
      
      CID 160637
      ----------
      The IS_VALID_OLD_STATE_FILEREP(state) macro is defined as:
      
          (state >= FILEREP_PUS_MUS && state <= FILEREP_PUC_MDX)
      
      where the FILEREP_* states are enums starting at 0.
      
      The macro is called with state defined as uint32.  This results in the following
      Coverity warning.
      
      Macro compares unsigned to 0 (NO_EFFECT)
      unsigned_compare: This greater-than-or-equal-to-zero comparison of an unsigned
      value is always true. stateOld >= FILEREP_PUS_MUS
      
      I have modified the macro definition to:
      
          ((unsigned int)(state) <= FILEREP_PUC_MDX)
      
      160639
      ------
      The IS_VALID_NEW_STATE_FILEREP(state) macro is defined as:
      
          (state >= FILEREP_PUS_MUS && state < FILEREP_SENTINEL)
      
      where the FILEREP_* states are enums starting at 0.
      
      The macro is called with state defined as uint32.  This results in the following
      Coverity warning.
      
      Macro compares unsigned to 0 (NO_EFFECT)
      unsigned_compare: This greater-than-or-equal-to-zero comparison of an unsigned
      value is always true. pairState->stateNew >= FILEREP_PUS_MUS.
      
      I have modified the macro definition to:
      
          ((unsigned int)(state) < FILEREP_SENTINEL)
      
      ftsprobe.c
      ==========
      
      CID 129898
      ----------
      In function probeTimeout(), we have:
      
      unint64 elapsed_ms = ...;
      
      if (elapsed_ms > gp_fts_probe_timeout * 1000)
        ...
      
      gp_fts_probe_timout is defined as an int.  Therefore, we get the following
      Coverity error:
      
      Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN)
      overflow_before_widen: Potentially overflowing expression gp_fts_probe_timeout *
      1000 with type int (32 bits, signed) is evaluated using 32-bit arithmetic, and
      then used in a context that expects an expression of type uint64 (64 bits,
      unsigned).
      
      I have casted gp_fts_probe_timout to uint64 to resolve this.
      
      primary_mirror_mode.c
      =====================
      
      CID 160714
      ----------
      Function probeProcessResponse() in ftsprobe.c reads a network packet to process
      probe response from the segments.  The fault field from the packet is passed to
      getFaultType() in primary_mirror_mode.c to index the labels array.  Therefore,
      we get the following warning from Coverity.
      
      Use of untrusted scalar value (TAINTED_SCALAR)
      tainted_data: Passing tainted variable fault to a tainted sink.
      
      I resolved this my making sure that the faultType is within the dimension
      of the labels array before using the faultType as an index.
      
      ftssan.c
      =========
      
      CID 160620
      ----------
      Added missing break in FtsResolveStateSAN() for the SAN_PU_MU case.
      
      CID 160636
      ----------
      The IS_VALID_NEW_STATE_SN(state) macro is defined as:
      
          (state >= SAN_PU_MU && state < SAN_SENTINEL)
      
      where the SAN_* states are enums starting at 0.
      
      The macro is called with state defined as uint32.  This results in the following
      Coverity warning.
      
      Macro compares unsigned to 0 (NO_EFFECT)
      unsigned_compare: This greater-than-or-equal-to-zero comparison of an unsigned
      value is always true. pairState->stateNew >= SAN_PU_MU.
      
      I have modified the macro definition to:
      
          ((unsigned int)(state) < SAN_SENTINEL)
      
      CID 160638
      -----------
      The IS_VALID_OLD_STATE_SAN(state) macro is defined as:
      
          (state >= SAN_PU_MU && state <= SAN_PU_MD)
      
      where the SAN_* states are enums starting at 0.
      
      The macro is called with state defined as uint32.  This results in the following
      Coverity warning.
      
      Macro compares unsigned to 0 (NO_EFFECT)
      unsigned_compare: This greater-than-or-equal-to-zero comparison of an unsigned
      value is always true. stateOld >= SAN_PU_MU.
      
      I have modified the macro definition to:
      
          ((unsigned int)(state) <= SAN_PU_MD)
      
      CID 157330
      ----------
      
      The mountMaintenanceForMpid() function sets the gphome_env variable to
      getenv("GPHOME").  The gphome_env variable is used as a file path in a command
      string that is passed to the system() OS call.  This results in the following
      warning from Coverity.
      
      Use of untrusted string value (TAINTED_STRING)
      tainted_string: Passing tainted string cmd to system, which cannot accept
      tainted data.
      
      I resolved this by using the execlp() system call instead of system() as
      recommended in
      https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=2130132.
      
      To use execlp(), individual arguments have to be built as separate characters
      strings.
      df5460ea
  26. 28 10月, 2015 1 次提交