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 , 8 years ago
comment:2 by , 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
1461 1461 } 1462 1462 1463 1463 /************************************************************************/ 1464 /* C SLTestBoolean()*/1464 /* CPLTestBool() */ 1465 1465 /************************************************************************/ 1466 1466 1467 1467 /** 1468 1468 * Test what boolean value contained in the string. 1469 1469 * 1470 * If pszValue is "NO", "FALSE", "OFF" or "0" will be returned FALSE.1471 * Otherwise, TRUEwill be returned.1470 * If pszValue is "NO", "FALSE", "OFF" or "0" will be returned false. 1471 * Otherwise, true will be returned. 1472 1472 * 1473 1473 * @param pszValue the string should be tested. 1474 1474 * … … 1475 1475 * @return TRUE or FALSE. 1476 1476 */ 1477 1477 1478 int CSLTestBoolean( const char *pszValue )1478 bool CPLTestBool( const char *pszValue ) 1479 1479 { 1480 // TODO: Simplify to return !(...). 1480 1481 if( EQUAL(pszValue,"NO") 1481 1482 || EQUAL(pszValue,"FALSE") 1482 1483 || EQUAL(pszValue,"OFF") 1483 1484 || EQUAL(pszValue,"0") ) 1484 return FALSE;1485 return false; 1485 1486 1486 return TRUE;1487 return true; 1487 1488 } 1488 1489 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 1507 int CSLTestBoolean( const char *pszValue ) 1508 { 1509 return CPLTestBool( pszValue ) ? TRUE : FALSE; 1510 } 1511 1489 1512 /********************************************************************** 1490 1513 * CSLFetchBoolean() 1491 1514 * -
cpl_string.h
103 103 int CPL_DLL CSLFetchBoolean( char **papszStrList, const char *pszKey, 104 104 int bDefault ); 105 105 106 #ifdef __cplusplus 107 /* Prefer these for C++ code. */ 108 bool CPLTestBool( const char *pszValue ); 109 #endif /* __cplusplus */ 110 106 111 const char CPL_DLL * 107 112 CPLParseNameValue(const char *pszNameValue, char **ppszKey ); 108 113 const char CPL_DLL *
comment:5 by , 8 years ago
Next tasks:
- Create a CPLTestBoolean that internally calls CPLTestBool.
- Change CSLTestBoolean -> CPLTestBoolean in source:trunk/gdal/frmts/bsb/bsb_read.c and source:trunk/gdal/frmts/nitf/nitfimage.c
- Add a comment that CSLTestBoolean is deprecated and will be removed in GDAL 3.x
- Convert the >500+ c++ calls of CSLTestBoolean -> CPLTestBool
comment:6 by , 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 , 8 years ago
comment:8 by , 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:10 by , 5 years ago
Milestone: | → closed_because_of_github_migration |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
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.
Looks good to me. CPLTestBool() would indeed makes more sense than CSLTestBool(). Not sure if there was a particular reason for the CSLTestBoolean naming.