Skip to content

[ogr2ogr][vrt] Do not return 0 on errors in the VRT#14839

Open
elpaso wants to merge 2 commits into
OSGeo:masterfrom
elpaso:bugfix-gh_14826-ogr2ogr-invalid-vrt
Open

[ogr2ogr][vrt] Do not return 0 on errors in the VRT#14839
elpaso wants to merge 2 commits into
OSGeo:masterfrom
elpaso:bugfix-gh_14826-ogr2ogr-invalid-vrt

Conversation

@elpaso

@elpaso elpaso commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Fix #14826

Check if there were errors opening the VRT and error out if that happens.

@elpaso elpaso added the funded through GSP Work funded through the GDAL Sponsorship Program label Jun 22, 2026
Fix OSGeo#14826

Check if there were errors opening the VRT and error
out if that happens.

Also catch errors from write alg
@elpaso elpaso force-pushed the bugfix-gh_14826-ogr2ogr-invalid-vrt branch from 4a1587b to 04f8b46 Compare June 22, 2026 12:44
@rouault rouault added the backport release/3.13 Backport to release/3.13 label Jun 22, 2026
bChangeGeomType, eType, m_overrideCrs,
m_layerMetadata, m_unsetLayerMetadata,
m_unsetFID));
if (CPLGetErrorCounter() != errorCount)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this convert warnings into failures? It looks like CPLErrorV increments the error counter whenever it is called, regardless of the error type?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I missed during review that GDALVectorEditAlgorithmLayer() constrructor emits warnings. We should indeed restrict the before/after check just around oSrcLayer.GetLayerDefn()

@dbaston dbaston Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a static std::unique_ptr<GDALVectorEditAlgorithmLayer>::Create that would return a nullptr on construction failure, allowing us to use conventional return code error propagation?

Or GetLayerDefn() returning nullptr on failure?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or GetLayerDefn() returning nullptr on failure?

Actually that was my first attempt but after a conversation with @rouault we decided that GDAL source code is not ready for GetLayerDefn() returning nullptr on failure.

I'll check the alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport release/3.13 Backport to release/3.13 funded through GSP Work funded through the GDAL Sponsorship Program

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ogr2ogr returns exit code 0 on error when source is VRT pointing to unreachable WFS

3 participants