From: Alberto Leiva Popper Date: Mon, 6 Nov 2023 17:39:12 +0000 (-0600) Subject: Delete struct snapshot and struct delta X-Git-Tag: 1.6.0~29 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=dc77cf179be4b0c4b2efb532c8d73be4c6587ca1;p=thirdparty%2FFORT-validator.git 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 48789ea9..6989ef86 100644 --- 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 5ded377e..3a15f2c8 100644 --- 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 a0b3222b..cbadb943 100644 --- 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) ¬if->snapshot); } else if (xmlStrEqual(name, BAD_CAST RRDP_ELEM_NOTIFICATION)) { /* No need to validate session ID and serial */ - return parse_metadata(reader, ¬if->meta, NULL, 0); + return parse_metadata(reader, ¬if->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 0a663475..f497cdcb 100644 --- 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