Tarsnap / tarsnap

Command-line client code for Tarsnap.
https://tarsnap.com
Other
871 stars 60 forks source link

storage_read_cache prep #503

Closed gperciva closed 2 years ago

gperciva commented 2 years ago

Some tidbits:

$ git show --ignore-space-change a867831
...
diff --git a/tar/storage/storage_read.c b/tar/storage/storage_read.c
index 8d15152..225a397 100644
--- a/tar/storage/storage_read.c
+++ b/tar/storage/storage_read.c
@@ -549,10 +549,11 @@ callback_read_file_response(void * cookie, NETPACKET_CONNECTION * NPC,
                }

                /* Should we cache this data? */
+               if (sc == 0) {
                        classname[0] = C->class;
                        memcpy(&classname[1], C->name, 32);
                        if (((CF = rwhashtab_read(C->S->cache_ht, classname)) != NULL) &&
-                   (CF->inqueue != 0) && (CF->buf == NULL) && (sc == 0)) {
+                           (CF->inqueue != 0) && (CF->buf == NULL)) {
                                /* Make a copy of this buffer if we can. */
                                if ((CF->buf = malloc(C->buflen)) != NULL) {
                                        /* Copy in data and data length. */
@@ -564,6 +565,7 @@ callback_read_file_response(void * cookie, NETPACKET_CONNECTION * NPC,
                                }
                        }
                }
+       }

        /*
         * If the user's tarsnap account balance is negative, print a warning
gperciva commented 2 years ago

I've reconsidered having the pair of:

storage_read_add_name_cache()
storage_read_add_data_cache()

We need a public API function for storage_read_add_name_cache() -- with potentially a different name, like storage_read_request_to_cache() or storage_read_add_data_for_this_name_to_the_cache_if_you_see_it() -- but we don't need a public function for _add_data_cache().

With that in mind, I'd suggest that we name the latter function what it'll end up being inside storage_read_cache.h... probably storage_read_cache_add_data(), unless a better name comes to mind.

gperciva commented 2 years ago

Rebased for review, updated the top comment. PR 1/3 towards an independent storage_read_cache.[c,h].

cperciva commented 2 years ago

Ok, LGTM.

gperciva commented 2 years ago

Rebased; this changed the history of the last 3 commits of this PR.