Have --catch-up=SESSION add the new hash files to SESSION#5966
Have --catch-up=SESSION add the new hash files to SESSION#5966magnumripper wants to merge 1 commit into
Conversation
583cdd4 to
f379506
Compare
|
This is tested in all relevant ways I could think of, such as with |
6b3cdd9 to
b3f441c
Compare
afdfa88 to
8c322b4
Compare
After the --catch-up=SESSION run caught up, its input files are now added to SESSION's recovery file so that it can simply be resumed for continuing with its old files as well as the new ones. This can be disabled with a new --no-catch-up-add option.
8c322b4 to
8369e88
Compare
| {"restore", FLG_RESTORE_SET, FLG_RESTORE_CHK, 0, ~FLG_RESTORE_SET & ~GETOPT_FLAGS, OPT_FMT_STR_ALLOC, &options.session}, | ||
| {"session", FLG_SESSION, FLG_SESSION, FLG_CRACKING_SUP, OPT_REQ_PARAM, OPT_FMT_STR_ALLOC, &options.session}, | ||
| {"catch-up", FLG_ONCE, 0, 0, OPT_REQ_PARAM, OPT_FMT_STR_ALLOC, &options.catchup}, | ||
| {"catch-up-add", FLG_ONCE, 0, 0, USUAL_REQ_CLR | OPT_TRISTATE, NULL, &options.catchup_add}, |
There was a problem hiding this comment.
Why is this OPT_TRISTATE? (Just a question, not implying there's anything wrong.)
solardiz
left a comment
solardiz
left a comment
There was a problem hiding this comment.
So I thought of just merging this quickly, but then I saw it's complicated. We can still just merge or we can discuss first... which I guess neither of us has much time for now - I certainly don't. What do you suggest?
| file so you can just resume that session. This will end up just as if both | ||
| sets of hashes were in the original session to start with, with no wasted | ||
| resources or time (assuming salted hashes). If you do not wish the original | ||
| session file to be touched, also add the --no-catch-up-add option. |
There was a problem hiding this comment.
Just noting that we don't have the --no-catch-up-add option documented in its own section in OPTIONS, we just mention it here. This may be fine. Maybe we already do similar for other obscure options?
| return ret; | ||
| } | ||
|
|
||
| void rec_add_files(char *session) |
There was a problem hiding this comment.
This rec_add_files is quite some code and quite some duplication of logic (filename suffix, locking) with our existing rec file writing code. As an alternative, maybe we could be adding the extra files to rec_argv and rec_argc, and then our existing code would just happen to write them into the rec file at next sync point (which we could trigger explicitly if needed).
| fprintf(catchup_file, "%s\n", current->data); | ||
|
|
||
| /* Append original tail */ | ||
| fprintf(catchup_file, "%s", catchup_data + offset); |
There was a problem hiding this comment.
Maybe use fwrite. We read this data with fread.
|
|
||
| /* Append original tail */ | ||
| fprintf(catchup_file, "%s", catchup_data + offset); | ||
| fclose(catchup_file); |
There was a problem hiding this comment.
In our usual code, we don't just fclose. We check with ferror first (to detect any errors of fprintf), then fflush (with its return value checked), ftruncate (ditto), and fsync (ditto). OK, ftruncate may be unneeded here if we're sure we can only increase the size, but the rest are probably good to have?
There was a problem hiding this comment.
Regarding "sure we can only increase the size", I'm confused as to whether we're rewriting the same file we read from, or some other file. If it's a different file, then its previous content may have been unrelated and larger, and then we'd need to truncate.
There was a problem hiding this comment.
Oh, indeed same file - this code literally seeks to offset 0 again. Then maybe we're fine without truncation.
| if (fclose(rec_file)) | ||
| pexit("fclose"); | ||
| rec_file = NULL; | ||
| } |
There was a problem hiding this comment.
I'm confused - why do we delete and close this session's file? I don't seem to fully understand the logic this implements. Also, what would these two filenames be in typical usage?
2 participants
After the --catch-up=SESSION run finished, add its input files to SESSION's recovery file so that SESSION can simply be resumed for continuing with its old files as well as the new ones.
This can be disabled with a new --no-catch-up-add option.