Opened 8 years ago

Closed 5 years ago

#6294 closed enhancement (wontfix)

Add matching functions that return C++ bool for boolean functions that return int.

Reported by: Kurt Schwehr Owned by: Kurt Schwehr
Priority: normal Milestone: closed_because_of_github_migration
Component: default Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

I propose that we add matching functions and methods to ports/cpl* for current items that return TRUE/FALSE via an int. Something like this with cpl_string.h as an example:

int CSLTestBoolean( const char *pszValue );
int CSLFetchBoolean( char **papszStrList, const char *pszKey,  int bDefault );

Would also have:

#ifdef __cplusplus
bool CSLTestBool( const char *pszValue );
bool CSLFetchBool( char **papszStrList, const char * pszKey,  bool bDefault );
#endif  /* __cplusplus */

The motivation would be:

  • Help static analizers, optimizers and fuzzers to better reduce the state space they are reasoning about.
  • Make cases where the int is being used for more than TRUE/FALSE more obvious. e.g. -1 being used for unknown.

Questions:

  • Why does CSLTestBoolean have CSL when it does not involve a string list?
  • Should we change make the new C++ function for CSLTestBoolean be CPLTestBool?
  • Should some other naming convention be used?

Change History (10)

comment:1 by Even Rouault, 8 years ago

Looks good to me. CPLTestBool() would indeed makes more sense than CSLTestBool(). Not sure if there was a particular reason for the CSLTestBoolean naming.

comment:2 by Kurt Schwehr, 8 years ago

Here is the patch that I have going at the moment. Is the ?: good or bad in the rewritten body of CSLTestBoolean()?

  • cpl_string.cpp

    svn diff
     
    14611461}
    14621462
    14631463/************************************************************************/
    1464 /*                         CSLTestBoolean()                             */
     1464/*                         CPLTestBool()                                */
    14651465/************************************************************************/
    14661466
    14671467/**
    14681468 * Test what boolean value contained in the string.
    14691469 *
    1470  * If pszValue is "NO", "FALSE", "OFF" or "0" will be returned FALSE.
    1471  * Otherwise, TRUE will be returned.
     1470 * If pszValue is "NO", "FALSE", "OFF" or "0" will be returned false.
     1471 * Otherwise, true will be returned.
    14721472 *
    14731473 * @param pszValue the string should be tested.
    14741474 *
     
    14751475 * @return TRUE or FALSE.
    14761476 */
    14771477
    1478 int CSLTestBoolean( const char *pszValue )
     1478bool CPLTestBool( const char *pszValue )
    14791479{
     1480  // TODO: Simplify to return !(...).
    14801481    if( EQUAL(pszValue,"NO")
    14811482        || EQUAL(pszValue,"FALSE")
    14821483        || EQUAL(pszValue,"OFF")
    14831484        || EQUAL(pszValue,"0") )
    1484         return FALSE;
     1485        return false;
    14851486
    1486     return TRUE;
     1487    return true;
    14871488}
    14881489
     1490/************************************************************************/
     1491/*                         CSLTestBoolean()                             */
     1492/************************************************************************/
     1493
     1494/**
     1495 * Test what boolean value contained in the string.
     1496 *
     1497 * If pszValue is "NO", "FALSE", "OFF" or "0" will be returned FALSE.
     1498 * Otherwise, TRUE will be returned.
     1499 *
     1500 * Use this only in C code.  In C++, prefer CPLTestBool().
     1501 *
     1502 * @param pszValue the string should be tested.
     1503 *
     1504 * @return TRUE or FALSE.
     1505 */
     1506
     1507int CSLTestBoolean( const char *pszValue )
     1508{
     1509    return CPLTestBool( pszValue ) ? TRUE : FALSE;
     1510}
     1511
    14891512/**********************************************************************
    14901513 *                       CSLFetchBoolean()
    14911514 *
  • cpl_string.h

     
    103103int CPL_DLL CSLFetchBoolean( char **papszStrList, const char *pszKey,
    104104                             int bDefault );
    105105
     106#ifdef __cplusplus
     107/* Prefer these for C++ code. */
     108bool CPLTestBool( const char *pszValue );
     109#endif  /* __cplusplus */
     110
    106111const char CPL_DLL *
    107112      CPLParseNameValue(const char *pszNameValue, char **ppszKey );
    108113const char CPL_DLL *

comment:3 by Even Rouault, 8 years ago

Looks good to me

comment:4 by Kurt Schwehr, 8 years ago

CPLTestBool() submitted to trunk in r32786

comment:5 by Kurt Schwehr, 8 years ago

Next tasks:

Error: Failed to load processor bash
No macro or processor named 'bash' found

comment:6 by Kurt Schwehr, 8 years ago

r32980 tries out the cleanup on NetCDF small before doing many more. I expect to get a number of MSVC failures with compilation on https://ci.appveyor.com/project/rouault/gdal-coverage/history during this process as I am trying to remove some of the now redundant CPL_TO_BOOL calls.

comment:7 by Kurt Schwehr, 8 years ago

CSLTestBoolean -> CPLTestBool: r32981, r32982, r32983, r32984, r32988, r32989, r32990, r32991

Next up:

CSLFetchBoolean -> CSLFetchBool.

This might be tricky to watch out for cases where the default value is not TRUE or FALSE.

find . -name \*.cpp | xargs grep CSLTestBoolean | wc -l
      55

comment:8 by Raeburn, 8 years ago

Perhaps I could work on the enhancement stated by goatbar in the last comment(comment 7)? I am quite new to open source, and I know C++.

comment:9 by Even Rouault, 8 years ago

Milestone: 2.1.0

Unmilestoning

comment:10 by Even Rouault, 5 years ago

Milestone: closed_because_of_github_migration
Resolution: wontfix
Status: newclosed

This ticket has been automatically closed because Trac is no longer used for GDAL bug tracking, since the project has migrated to GitHub. If you believe this ticket is still valid, you may file it to https://github.com/OSGeo/gdal/issues if it is not already reported there.

Note: See TracTickets for help on using tickets.