WT-1939 Improve error handling in example code #3467
Conversation
Rework error handling in example code so we always check return values, but without cluttering the documentation.
[-Werror=declaration-after-statement]
This reverts commit 90339af.
There was a problem hiding this comment.
@keithbostic These changes look great to me. I had one comment, and I fixed the join example in ex_stat.c
| char cmd_buf[256]; | ||
|
|
||
| (void)argc; /* Unused variable */ | ||
| (void)testutil_set_progname(argv); | ||
|
|
||
| (void)snprintf(cmd_buf, sizeof(cmd_buf), | ||
| "rm -rf %s && mkdir %s", home, home); |
There was a problem hiding this comment.
Could this be replaced by home = example_setup(argc, argv); like other examples?
There was a problem hiding this comment.
I think so, but I'll leave that up to @sueloverso: ex_backup.c has its own set of 3 databases, and the naming is clearly intended to align between the 3, so I was hesitant to change it.
There was a problem hiding this comment.
That test does a lot of directory manipulation. It actually uses a larger number as it creates a differently numbered incremental database directory for each successive pass. I suggest leaving it alone and it is likely a one-off and its usage is not worth making into a higher level function.
|
Thanks @keithbostic I really like the direction you went with this change. FYI: Sue is on vacation this week - so this is likely to sit until she gets back. |
|
@keithbostic I pushed a fix for the |
examples/c/ex_call_center.c
Outdated
| } | ||
| /* Note: further error checking omitted for clarity. */ | ||
| home = example_setup(argc, argv); | ||
|
|
There was a problem hiding this comment.
very minor nit: since home is used in the next line, I'd remove the blank line.
@agorrod, @sueloverso: I took a run at cleaning up the example code error handling.
I hope this resolves WT-3364 and WT-1939.
I ended up using Alex's technique from the test/utility code, adding two macros: first, a macro
error_checkthat checks for a non-zero return from a function call, and second, a macroscan_end_checkwhich checks thatret == WT_NOTFOUNDafter a cursor scan completes.I decided against adding explicit error checking with messages because I doubt our ability to ever exercise those error message paths and the error handling messages clutters the documentation more than I wanted.
I chose the names
error_checkandscan_end_checkto hopefully make it clear in the documentation what's going on, happy to switch to something else if anybody has a better idea.I found and fixed a few places in the example code where we weren't checking error returns and the example code fails, but there were two I didn't fix:
session->transaction_synccall inex_sync.cthat returns failure, can you please take a look and fix it?join_cursor->nextcall inex_stat.cthat returns failure, can you please take a look and fix it?Both of those calls are currently commented out so the tests pass, just search for
KEITHin the file to find the call.