Skip to content

Commit

Permalink
[Lint] VRT: remove use of magic constant -1.0 for uinitialized src/ds…
Browse files Browse the repository at this point in the history 
…t window
  • Loading branch information
@rouault
rouault committed Aug 13, 2024
1 parent 4952bc2 commit 4d4a47c
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 39 deletions.
38 changes: 29 additions & 9 deletions frmts/vrt/vrtdataset.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1236,15 +1236,21 @@ class CPL_DLL VRTSimpleSource CPL_NON_FINAL : public VRTSource
int m_nBand = 0;
bool m_bGetMaskBand = false;

double m_dfSrcXOff = 0;
double m_dfSrcYOff = 0;
double m_dfSrcXSize = 0;
double m_dfSrcYSize = 0;

double m_dfDstXOff = 0;
double m_dfDstYOff = 0;
double m_dfDstXSize = 0;
double m_dfDstYSize = 0;
/* Value for uninitialized source or destination window. It is chosen such
* that SrcToDst() and DstToSrc() are no-ops if both source and destination
* windows are unset.
*/
static constexpr double UNINIT_WINDOW = -1.0;

double m_dfSrcXOff = UNINIT_WINDOW;
double m_dfSrcYOff = UNINIT_WINDOW;
double m_dfSrcXSize = UNINIT_WINDOW;
double m_dfSrcYSize = UNINIT_WINDOW;

double m_dfDstXOff = UNINIT_WINDOW;
double m_dfDstYOff = UNINIT_WINDOW;
double m_dfDstXSize = UNINIT_WINDOW;
double m_dfDstYSize = UNINIT_WINDOW;

CPLString m_osResampling{};

Expand Down Expand Up @@ -1275,6 +1281,20 @@ class CPL_DLL VRTSimpleSource CPL_NON_FINAL : public VRTSource
return true;
}

/** Returns whether the source window is set */
bool IsSrcWinSet() const
{
return m_dfSrcXOff != UNINIT_WINDOW || m_dfSrcYOff != UNINIT_WINDOW ||
m_dfSrcXSize != UNINIT_WINDOW || m_dfSrcYSize != UNINIT_WINDOW;
}

/** Returns whether the destination window is set */
bool IsDstWinSet() const
{
return m_dfDstXOff != UNINIT_WINDOW || m_dfDstYOff != UNINIT_WINDOW ||
m_dfDstXSize != UNINIT_WINDOW || m_dfDstYSize != UNINIT_WINDOW;
}

public:
VRTSimpleSource();
VRTSimpleSource(const VRTSimpleSource *poSrcSource, double dfXDstRatio,
Expand Down
67 changes: 37 additions & 30 deletions frmts/vrt/vrtsources.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,7 @@ CPLXMLNode *VRTSimpleSource::SerializeToXML(const char *pszVRTPath)
CPLSPrintf("%d", nBlockYSize));
}

if (m_dfSrcXOff != -1 || m_dfSrcYOff != -1 || m_dfSrcXSize != -1 ||
m_dfSrcYSize != -1)
if (IsSrcWinSet())
{
CPLSetXMLValue(psSrc, "SrcRect.#xOff",
CPLSPrintf("%.15g", m_dfSrcXOff));
Expand All @@ -483,8 +482,7 @@ CPLXMLNode *VRTSimpleSource::SerializeToXML(const char *pszVRTPath)
CPLSPrintf("%.15g", m_dfSrcYSize));
}

if (m_dfDstXOff != -1 || m_dfDstYOff != -1 || m_dfDstXSize != -1 ||
m_dfDstYSize != -1)
if (IsDstWinSet())
{
CPLSetXMLValue(psSrc, "DstRect.#xOff",
CPLSPrintf("%.15g", m_dfDstXOff));
Expand Down Expand Up @@ -581,21 +579,30 @@ VRTSimpleSource::XMLInit(const CPLXMLNode *psSrc, const char *pszVRTPath,

CPLErr VRTSimpleSource::ParseSrcRectAndDstRect(const CPLXMLNode *psSrc)
{
const auto GetAttrValue = [](const CPLXMLNode *psNode,
const char *pszAttrName, double dfDefaultVal)
{
if (const char *pszVal = CPLGetXMLValue(psNode, pszAttrName, nullptr))
return CPLAtof(pszVal);
else
return dfDefaultVal;
};

/* -------------------------------------------------------------------- */
/* Set characteristics. */
/* -------------------------------------------------------------------- */
const CPLXMLNode *const psSrcRect = CPLGetXMLNode(psSrc, "SrcRect");
if (psSrcRect)
{
double xOff = CPLAtof(CPLGetXMLValue(psSrcRect, "xOff", "-1"));
double yOff = CPLAtof(CPLGetXMLValue(psSrcRect, "yOff", "-1"));
double xSize = CPLAtof(CPLGetXMLValue(psSrcRect, "xSize", "-1"));
double ySize = CPLAtof(CPLGetXMLValue(psSrcRect, "ySize", "-1"));
double xOff = GetAttrValue(psSrcRect, "xOff", UNINIT_WINDOW);
double yOff = GetAttrValue(psSrcRect, "yOff", UNINIT_WINDOW);
double xSize = GetAttrValue(psSrcRect, "xSize", UNINIT_WINDOW);
double ySize = GetAttrValue(psSrcRect, "ySize", UNINIT_WINDOW);
// Test written that way to catch NaN values
if (!(xOff >= INT_MIN && xOff <= INT_MAX) ||
!(yOff >= INT_MIN && yOff <= INT_MAX) ||
!(xSize > 0 || xSize == -1) || xSize > INT_MAX ||
!(ySize > 0 || ySize == -1) || ySize > INT_MAX)
!(xSize > 0 || xSize == UNINIT_WINDOW) || xSize > INT_MAX ||
!(ySize > 0 || ySize == UNINIT_WINDOW) || ySize > INT_MAX)
{
CPLError(CE_Failure, CPLE_AppDefined, "Wrong values in SrcRect");
return CE_Failure;
Expand All @@ -604,24 +611,26 @@ CPLErr VRTSimpleSource::ParseSrcRectAndDstRect(const CPLXMLNode *psSrc)
}
else
{
m_dfSrcXOff = -1;
m_dfSrcYOff = -1;
m_dfSrcXSize = -1;
m_dfSrcYSize = -1;
m_dfSrcXOff = UNINIT_WINDOW;
m_dfSrcYOff = UNINIT_WINDOW;
m_dfSrcXSize = UNINIT_WINDOW;
m_dfSrcYSize = UNINIT_WINDOW;
}

const CPLXMLNode *const psDstRect = CPLGetXMLNode(psSrc, "DstRect");
if (psDstRect)
{
double xOff = CPLAtof(CPLGetXMLValue(psDstRect, "xOff", "-1"));
double yOff = CPLAtof(CPLGetXMLValue(psDstRect, "yOff", "-1"));
double xSize = CPLAtof(CPLGetXMLValue(psDstRect, "xSize", "-1"));
double ySize = CPLAtof(CPLGetXMLValue(psDstRect, "ySize", "-1"));
double xOff = GetAttrValue(psDstRect, "xOff", UNINIT_WINDOW);
;
double yOff = GetAttrValue(psDstRect, "yOff", UNINIT_WINDOW);
double xSize = GetAttrValue(psDstRect, "xSize", UNINIT_WINDOW);
;
double ySize = GetAttrValue(psDstRect, "ySize", UNINIT_WINDOW);
// Test written that way to catch NaN values
if (!(xOff >= INT_MIN && xOff <= INT_MAX) ||
!(yOff >= INT_MIN && yOff <= INT_MAX) ||
!(xSize > 0 || xSize == -1) || xSize > INT_MAX ||
!(ySize > 0 || ySize == -1) || ySize > INT_MAX)
!(xSize > 0 || xSize == UNINIT_WINDOW) || xSize > INT_MAX ||
!(ySize > 0 || ySize == UNINIT_WINDOW) || ySize > INT_MAX)
{
CPLError(CE_Failure, CPLE_AppDefined, "Wrong values in DstRect");
return CE_Failure;
Expand All @@ -630,10 +639,10 @@ CPLErr VRTSimpleSource::ParseSrcRectAndDstRect(const CPLXMLNode *psSrc)
}
else
{
m_dfDstXOff = -1;
m_dfDstYOff = -1;
m_dfDstXSize = -1;
m_dfDstYSize = -1;
m_dfDstXOff = UNINIT_WINDOW;
m_dfDstYOff = UNINIT_WINDOW;
m_dfDstXSize = UNINIT_WINDOW;
m_dfDstYSize = UNINIT_WINDOW;
}

return CE_None;
Expand Down Expand Up @@ -807,7 +816,7 @@ int VRTSimpleSource::IsSameExceptBandNumber(VRTSimpleSource *poOtherSource)
/************************************************************************/
/* SrcToDst() */
/* */
/* Note: this is a no-op if the dst window is -1,-1,-1,-1. */
/* Note: this is a no-op if the both src and dst windows are unset */
/************************************************************************/

void VRTSimpleSource::SrcToDst(double dfX, double dfY, double &dfXOut,
Expand All @@ -821,7 +830,7 @@ void VRTSimpleSource::SrcToDst(double dfX, double dfY, double &dfXOut,
/************************************************************************/
/* DstToSrc() */
/* */
/* Note: this is a no-op if the dst window is -1,-1,-1,-1. */
/* Note: this is a no-op if the both src and dst windows are unset */
/************************************************************************/

void VRTSimpleSource::DstToSrc(double dfX, double dfY, double &dfXOut,
Expand Down Expand Up @@ -852,12 +861,10 @@ int VRTSimpleSource::GetSrcDstWindow(
return FALSE;
}

const bool bDstWinSet = m_dfDstXOff != -1 || m_dfDstXSize != -1 ||
m_dfDstYOff != -1 || m_dfDstYSize != -1;
const bool bDstWinSet = IsDstWinSet();

#ifdef DEBUG
const bool bSrcWinSet = m_dfSrcXOff != -1 || m_dfSrcXSize != -1 ||
m_dfSrcYOff != -1 || m_dfSrcYSize != -1;
const bool bSrcWinSet = IsSrcWinSet();

if (bSrcWinSet != bDstWinSet)
{
Expand Down

0 comments on commit 4d4a47c

Please sign in to comment.