Stefan Beller [off-list ref] writes:
quoted
quoted
+static int delta_base_offset = 1;
+char *packdir;
Does this have to be global?
We could pass it to all the functions, making it not global.
Sorry for being unclear; I meant "not static". It is perfectly fine
for this to be a file-scope static.
quoted
quoted
+
+ if (!file_exists(mkpath("%s/%s.keep", packdir, fname)))
+ string_list_append_nodup(fname_list, fname);
mental note: this is getting names of non-kept packs, not all packs.
I should document that. ;)
Rather, consider giving the function a better name, perhaps?
quoted
quoted
+ while (strbuf_getline(&line, out, '\n') != EOF) {
+ if (line.len != 40)
+ die("repack: Expecting 40 character sha1 lines only from pack-objects.");
+ strbuf_addstr(&line, "");
What is this addstr() about?
According to the documentation of strbufs, we cannot assume to have sane
strings, but anything.
Sorry, I do not get this. What is a sane string and what is an
insane string? sb->buf[sb-len] is always terminated with a NUL
when strbuf_getline() returns success, isn't it?
On 08/22/2013 12:50 AM, Junio C Hamano wrote:
Stefan Beller [off-list ref] writes:
quoted
quoted
quoted
+static int delta_base_offset = 1;
+char *packdir;
Does this have to be global?
We could pass it to all the functions, making it not global.
Sorry for being unclear; I meant "not static". It is perfectly fine
for this to be a file-scope static.
No need to be sorry! I am sleepy, and may missunderstand even clear
messages. I'll change it to static of course.
quoted
quoted
quoted
+
+ if (!file_exists(mkpath("%s/%s.keep", packdir, fname)))
+ string_list_append_nodup(fname_list, fname);
mental note: this is getting names of non-kept packs, not all packs.
I should document that. ;)
Rather, consider giving the function a better name, perhaps?
What about one of:
get_non_kept_pack_filenames
get_prunable_pack_filenames
get_remove_candidate_pack_filenames
quoted
quoted
quoted
+ while (strbuf_getline(&line, out, '\n') != EOF) {
+ if (line.len != 40)
+ die("repack: Expecting 40 character sha1 lines only from pack-objects.");
+ strbuf_addstr(&line, "");
What is this addstr() about?
According to the documentation of strbufs, we cannot assume to have sane
strings, but anything.
Sorry, I do not get this. What is a sane string and what is an
insane string? sb->buf[sb-len] is always terminated with a NUL
when strbuf_getline() returns success, isn't it?
I should read the strbuf documentation again. Thanks for pointing it
out. I'll remove the strbuf_addstr(&line, "");
Thanks for your patience in the reviews,
Stefan