Delete struct snapshot and struct delta

Weird code design.

I originally meant to privatize and de-heap these structures,
but it turns out they were not just used by a single module;
they were only used by `parse_metadata()`.

(Their domain seemed larger than it was, because they were being
initialized elsewhere, for no apparent reason.)

Then, while trying to clean up the global namespace, I noticed that the
hack was actually intertwined with another one: `parse_metadata()` was
being used for two purposes that are never actually used simultaneously.
This cluttered it.

So also separate `parse_metadata()` from `validate_metadata()`.

diff --git a/src/rrdp/rrdp_objects.c b/src/rrdp/rrdp_objects.c
index 48789ea9d799a0e44b844672f06080109ba26acd..6989ef867803f999e91d2fffc12e63b9c55160f1 100644 (file)
--- a/src/rrdp/rrdp_objects.c
+++ b/src/rrdp/rrdp_objects.c
@@ -137,42 +137,6 @@ update_notification_cleanup(struct update_notification *file)
        uri_refput(file->uri);
 }
 
-struct snapshot *
-snapshot_create(void)
-{
-       struct snapshot *tmp;
-
-       tmp = pmalloc(sizeof(struct snapshot));
-       notification_metadata_init(&tmp->meta);
-
-       return tmp;
-}
-
-void
-snapshot_destroy(struct snapshot *file)
-{
-       notification_metadata_cleanup(&file->meta);
-       free(file);
-}
-
-struct delta *
-delta_create(void)
-{
-       struct delta *tmp;
-
-       tmp = pmalloc(sizeof(struct delta));
-       notification_metadata_init(&tmp->meta);
-
-       return tmp;
-}
-
-void
-delta_destroy(struct delta *file)
-{
-       notification_metadata_cleanup(&file->meta);
-       free(file);
-}
-
 void
 publish_init(struct publish *tag)
 {

diff --git a/src/rrdp/rrdp_objects.h b/src/rrdp/rrdp_objects.h
index 5ded377e37c0d55cb2d9686fd92ea8a7a3868f3a..3a15f2c81db7d198c8418ba6cda7800db323e501 100644 (file)
--- a/src/rrdp/rrdp_objects.h
+++ b/src/rrdp/rrdp_objects.h
@@ -28,22 +28,6 @@ struct withdraw {
        struct file_metadata meta;
 };
 
-/*
- * Delta file content.
- * Publish/withdraw list aren't remember, they are processed ASAP.
- */
-struct delta {
-       struct notification_metadata meta;
-};
-
-/*
- * Snapshot file content
- * Publish list isn't remember, is processed ASAP.
- */
-struct snapshot {
-       struct notification_metadata meta;
-};
-
 /* Delta element located at an update notification file */
 struct delta_head {
        /*
@@ -80,12 +64,6 @@ int deltas_head_for_each(struct deltas_head *, unsigned long, unsigned long,
     delta_head_cb, void *);
 int deltas_head_sort(struct deltas_head *, unsigned long);
 
-struct snapshot *snapshot_create(void);
-void snapshot_destroy(struct snapshot *);
-
-struct delta *delta_create(void);
-void delta_destroy(struct delta *);
-
 void publish_init(struct publish *);
 void publish_cleanup(struct publish *);
 

diff --git a/src/rrdp/rrdp_parser.c b/src/rrdp/rrdp_parser.c
index a0b3222b164725c0f0e3b927ec6543697be4fcfa..cbadb943a37e29ac7f0255d061fb15ac07427a45 100644 (file)
--- a/src/rrdp/rrdp_parser.c
+++ b/src/rrdp/rrdp_parser.c
@@ -32,16 +32,12 @@
 
 /* Context while reading a snapshot */
 struct rdr_snapshot_ctx {
-       /* Data being parsed */
-       struct snapshot *snapshot;
        /* Parent data to validate session ID and serial */
        struct update_notification *notif;
 };
 
 /* Context while reading a delta */
 struct rdr_delta_ctx {
-       /* Data being parsed */
-       struct delta *delta;
        /* Parent data to validate session ID */
        struct update_notification *notif;
        /* Current serial loaded from update notification deltas list */
@@ -283,13 +279,9 @@ validate_version(xmlTextReaderPtr reader, unsigned long expected)
        return 0;
 }
 
-/* @gdata elements are allocated */
 static int
-parse_metadata(xmlTextReaderPtr reader, struct notification_metadata *meta,
-    char const *expected_session, unsigned long expected_serial)
+parse_metadata(xmlTextReaderPtr reader, struct notification_metadata *meta)
 {
-       char *session_id;
-       unsigned long serial;
        int error;
 
        /*
@@ -306,42 +298,46 @@ parse_metadata(xmlTextReaderPtr reader, struct notification_metadata *meta,
        if (error)
                return error;
 
-       error = parse_string(reader, RRDP_ATTR_SESSION_ID, &session_id);
+       meta->serial = 0;
+       error = parse_long(reader, RRDP_ATTR_SERIAL, &meta->serial);
        if (error)
                return error;
 
-       serial = 0;
-       error = parse_long(reader, RRDP_ATTR_SERIAL, &serial);
-       if (error) {
-               free(session_id);
-               return error;
-       }
+       return parse_string(reader, RRDP_ATTR_SESSION_ID, &meta->session_id);
+}
+
+static int
+validate_metadata(xmlTextReaderPtr reader, char const *expected_session,
+    unsigned long expected_serial)
+{
+       struct notification_metadata meta;
+       int error;
 
-       if (expected_session == NULL)
-               goto return_val; /* Error O is OK */
+       notification_metadata_init(&meta);
 
-       /*
-        * Positive error value means the file was successfully parsed but it
-        * has a logic error (in this case, session ID doesn't match parent's).
-        */
-       if (strcmp(expected_session, session_id) != 0) {
+       error = parse_metadata(reader, &meta);
+       if (error)
+               return error;
+
+       if (strcmp(expected_session, meta.session_id) != 0) {
+               /* FIXME why are these not error messages */
                pr_val_info("File session id [%s] doesn't match parent's session id [%s]",
-                   expected_session, session_id);
-               error = EINVAL;
-               goto return_val;
+                   expected_session, meta.session_id);
+               goto fail;
        }
 
-       /* ...and the serial must match to what's expected at the parent */
-       if (serial != expected_serial) {
+       if (meta.serial != expected_serial) {
                pr_val_info("File serial '%lu' doesn't match expected serial '%lu'",
-                   serial, expected_serial);
-               error = EINVAL;
+                   meta.serial, expected_serial);
+               goto fail;
        }
 
-return_val:
-       meta->session_id = session_id;
-       meta->serial = serial;
-       return error;
+       notification_metadata_cleanup(&meta);
+       return 0;
+
+fail:
+       notification_metadata_cleanup(&meta);
+       return EINVAL;
 }
 
 /*
@@ -419,11 +415,6 @@ parse_publish(xmlTextReaderPtr reader, struct rpki_uri *notif,
 
        /* rfc8181#section-2.2 but considering optional hash */
        if (tag->meta.hash_len > 0) {
-               /*
-                * FIXME How come you're checking the hash of the file?
-                * You haven't written the file yet.
-                */
-
                /* Get the current file from the uri */
                error = hash_validate_file(tag->meta.uri, tag->meta.hash,
                    tag->meta.hash_len);
@@ -579,7 +570,7 @@ xml_read_notif(xmlTextReaderPtr reader, void *arg)
                            &notif->snapshot);
                } else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_NOTIFICATION)) {
                        /* No need to validate session ID and serial */
-                       return parse_metadata(reader, &notif->meta, NULL, 0);
+                       return parse_metadata(reader, &notif->meta);
                }
 
                return pr_val_err("Unexpected '%s' element", name);
@@ -625,8 +616,7 @@ xml_read_snapshot(xmlTextReaderPtr reader, void *arg)
                        error = parse_publish_elem(reader, ctx->notif->uri,
                            HR_IGNORE);
                else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_SNAPSHOT))
-                       error = parse_metadata(reader,
-                           &ctx->snapshot->meta,
+                       error = validate_metadata(reader,
                            ctx->notif->meta.session_id,
                            ctx->notif->meta.serial);
                else
@@ -655,12 +645,10 @@ parse_snapshot(struct rpki_uri *uri, struct update_notification *notif)
        if (error)
                goto pop;
 
-       ctx.snapshot = snapshot_create();
        ctx.notif = notif;
 
        error = relax_ng_parse(uri_get_local(uri), xml_read_snapshot, &ctx);
 
-       snapshot_destroy(ctx.snapshot);
 pop:   fnstack_pop();
        return error;
 }
@@ -683,8 +671,7 @@ xml_read_delta(xmlTextReaderPtr reader, void *arg)
                else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_WITHDRAW))
                        error = parse_withdraw_elem(reader, ctx->notif->uri);
                else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_DELTA))
-                       error = parse_metadata(reader,
-                           &ctx->delta->meta,
+                       error = validate_metadata(reader,
                            ctx->notif->meta.session_id,
                            ctx->expected_serial);
                else
@@ -716,13 +703,11 @@ parse_delta(struct rpki_uri *uri, struct delta_head *parents_data,
        if (error)
                goto pop_fnstack;
 
-       ctx.delta = delta_create();
        ctx.notif = notif;
        ctx.expected_serial = parents_data->serial;
 
        error = relax_ng_parse(uri_get_local(uri), xml_read_delta, &ctx);
 
-       delta_destroy(ctx.delta);
 pop_fnstack:
        fnstack_pop();
        return error;

diff --git a/test/cache/local_cache_test.c b/test/cache/local_cache_test.c
index 0a663475020a96c41e4558764f32ddb11ebd64f2..f497cdcbbbe23198cc346a127b4a7ba0cf535542 100644 (file)
--- a/test/cache/local_cache_test.c
+++ b/test/cache/local_cache_test.c
@@ -1336,6 +1336,8 @@ START_TEST(test_recover)
        ck_assert_ptr_eq(uris.array[14], cache_recover(&uris, false));
        uris_cleanup(&uris);
 
+       /* FIXME test HTTP (non-recursive) */
+
        cache_teardown();
 }
 END_TEST