[WIP] lib/repo: Validate config more strictly

[mwleeds]

Now that we're correctly reading negative values for the "lock-timeout-secs" config key, throw an error if it's not within the range of values that have semantic meaning. This mirrors the behavior when other config keys have invalid values.

[dbnicholson]

I like this better than relying on an assert since that's a programmer error, not a user error. Actually, there are a bunch of potential errors not caught here that were in my original implementation in https://github.com/ostreedev/ostree/pull/1292/commits/532199858ce283a7e8528a1f2affc7c8069a43fc. See also @pwithnall's review in https://github.com/ostreedev/ostree/pull/1292/commits/532199858ce283a7e8528a1f2affc7c8069a43fc#r147145444.

[mwleeds]

That link didn't work for me but I think I found what you're referring to here: https://github.com/ostreedev/ostree/pull/1292#discussion_r147145444

[cgwalters]

@rh-atomic-bot delegate=dbnicholson

[rh-atomic-bot]

:v: @dbnicholson can now approve this pull request

[pwithnall]

Unfortunately it does not look so good to all the compilers:

src/libostree/ostree-repo.c: In function 'reload_core_config':
src/libostree/ostree-repo.c:2810:11: error: unused variable 'endptr' [-Werror=unused-variable]
     char *endptr;
           ^~~~~~
src/libostree/ostree-repo.c:2840:9: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
         if (configured_lock_timeout < REPO_LOCK_DISABLED || configured_lock_timeout > G_MAXINT32 ||
         ^~
src/libostree/ostree-repo.c:2842:11: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
           return glnx_throw (error, "Invalid lock-timeout-secs '%s'", lock_timeout_seconds);
           ^~~~~~

[pwithnall]

Note: You could also potentially use g_ascii_string_to_[un]signed() rather than g_strto[u]ll(), if you depend on a new enough GLib version (I haven’ŧ checked).

[mwleeds]

Oops, didn't intend for y'all to spend time reviewing that last commit. I was pushing it to get it from one computer to another. But yeah, I'll follow-up and fix it.

[openshift-ci[bot]]

@mwleeds: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/sanity	dce18c9f10c18bacf6b7f33762d3b060c07c6928	link	true	/test sanity

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.