Opened 6 years ago

Closed 5 years ago

#7253 closed enhancement (wontfix)

API: OGRLayer::GetExtentInternal has useless 'bForce' parameter

Reported by: blarg Owned by: blarg
Priority: low Milestone: closed_because_of_github_migration
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

OGRLayer::GetExtentInternal has a 'bForce' parameter. If it is zero, then (after zeroing out some class members and validating something that looks like a class invariant), the method returns OGRERR_FAILURE to its caller.

The caller should not be depending on private class members, and should not worry about the class' invariants (they are the class's concern, not the caller's). So those steps can be ignored in this analysis.

This means that it's not useful to call this function with bForce == 0. All such a call would do is return the value OGRERR_FAILURE - but the caller could have obtained this value directly; there was no need to call a method on an instance of OGRLayer merely in order to obtain an error code.

Since the function is not useful to call with bForce == 0, all callers of it with bForce == 0 are buggy. The calls should all be replaced by the constant OGRERR_FAILURE. Fixing these bugs in application code is not GDAL's concern, so will not be considered further here.

Since the only valid use of this function is to call it with bForce != 0, bForce should default to non-zero, so that callers can eventually cease passing the pointless int argument.

Happily, bForce is currently the last parameter to the method, so it can be given a default value (which it currently lacks) without causing any backward compatibility problems.

There are several other similarly bizarre APIs in ogrlayer.cpp and ogrsf_frmts.h . It looks as if someone was trying to force all callers to say "no, I'm not calling this API accidentally; I really, really do want it to do what its documentation says it does".

The problem with this idea is that if an application can accidentally call this API, even though it didn't actually want to call it, then that application can also accidentally call it with bForce != 0.

GDAL can't protect application programmers from themselves.

Change History (1)

comment:1 by Even Rouault, 5 years ago

Milestone: closed_because_of_github_migration
Resolution: wontfix
Status: assignedclosed

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.