Skip to content
Snippets Groups Projects

Changeset refactor

Merged Ghost User requested to merge changeset-refactor into master

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 97 89
    98 90 /* Put REMOVE RRSets. */
    99 91 if (ixfr->state == IXFR_DEL) {
    100 ret = ixfr_put_rrlist(pkt, ixfr, &chgset->remove);
    92 if (EMPTY_LIST(ixfr->cur.iters) && knot_rrset_empty(&ixfr->cur_rr)) {
    93 changeset_iter_rem(&ixfr->cur, chgset, false);
    94 }
    95 ret = ixfr_put_chg_part(pkt, ixfr, &ixfr->cur);
    101 96 if (ret != KNOT_EOK) {
    102 97 return ret;
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 112 107 ixfr->state = IXFR_ADD;
    113 108 }
    114 109
    115 /* Put REMOVE RRSets. */
    110 /* Put Add RRSets. */
    116 111 if (ixfr->state == IXFR_ADD) {
    117 ret = ixfr_put_rrlist(pkt, ixfr, &chgset->add);
    112 if (EMPTY_LIST(ixfr->cur.iters) && knot_rrset_empty(&ixfr->cur_rr)) {
    113 changeset_iter_add(&ixfr->cur, chgset, false);
    114 }
    115 ret = ixfr_put_chg_part(pkt, ixfr, &ixfr->cur);
    118 116 if (ret != KNOT_EOK) {
    119 117 return ret;
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 352 347 }
    353 348
    354 349 /*! \brief Stores starting SOA into changesets structure. */
    355 static int solve_start(const knot_rrset_t *rr, changesets_t *changesets, mm_ctx_t *mm)
    350 static int solve_start(const knot_rrset_t *rr, struct ixfr_proc *proc)
    356 351 {
    357 assert(changesets->first_soa == NULL);
    352 assert(proc->final_soa == NULL);
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 24 24 /*! \brief Extended structure for IXFR-in/IXFR-out processing. */
    25 25 struct ixfr_proc {
    26 26 struct xfr_proc proc; /* Generic transfer processing context. */
    27 node_t *cur;
    27 changeset_iter_t cur; /* Current changeset iteration state.*/
    28 knot_rrset_t cur_rr; /* Currently processed RRSet. */
    28 29 int state; /* IXFR-in state. */
    29 changesets_t *changesets; /* Processed changesets. */
    30 knot_rrset_t *final_soa; /* First SOA received via IXFR. */
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 985 991 /* Remove RRSets. */
    986 992 if (in_remove_section) {
    987 ret = changeset_add_rrset(chs, rrset,
    988 CHANGESET_REMOVE);
    993 ret = changeset_rem_rrset(chs, &rrset);
    989 994 } else {
    990 995 /* Add RRSets. */
    991 ret = changeset_add_rrset(chs, rrset,
    992 CHANGESET_ADD);
    996 ret = changeset_add_rrset(chs, &rrset);
    993 997 }
    994 998 }
    999 knot_rrset_clear(&rrset, NULL);
    995 1000 }
    996 1001
    1002 knot_rrset_clear(&rrset, NULL);
    • Author Contributor

      This is needed only if the code in the previous while cycle is interrupted by the break - so it could have stayed in that else branch. In all other cases, this function is called on line 999.

    • Author Contributor

      Right. EDIT: :white_check_mark:

  • Ghost User
    Ghost User @ghost started a thread on the diff
  • Unable to load the diff
    • Author Contributor

      Return value is discarded. I know that the journal must be closed even if something went wrong before this, but either discard the return value explicitly, or better store it and return it if ret was previously EOK.

  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 1234 int journal_store_changeset(changeset_t *change, const char *path, size_t size_limit)
    1235 {
    1236 if (change == NULL || path == NULL) {
    1237 return KNOT_EINVAL;
    1238 }
    1239
    1240 /* Open journal for reading. */
    1241 journal_t *journal = journal_open(path, size_limit);
    1242 if (journal == NULL) {
    1243 return KNOT_ENOMEM;
    1244 }
    1213 1245
    1214 /* Written changesets to journal. */
    1215 return journal_close(journal);
    1246 int ret = changeset_pack(change, journal);
    1247
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 134 131 }
    135 132
    136 133 int rrset_deserialize(const uint8_t *stream, size_t *stream_size,
    137 knot_rrset_t **rrset)
    134 knot_rrset_t *rrset)
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 267 267 static int apply_add(zone_contents_t *contents, changeset_t *chset,
    268 268 bool master)
    • Author Contributor

      Isn't there a way to join this function and the previous one? They are quite similar, just the iterator initialization and the function for applying the RR differ.

    • Author Contributor

      What previous function do you mean?

  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 568 int apply_changeset_directly(zone_contents_t *contents, changeset_t *ch)
    521 569 {
    522 if (chgs == NULL) {
    523 return;
    570 if (contents == NULL || ch == NULL) {
    571 return KNOT_EINVAL;
    572 }
    573
    574 const bool master = true; // Only DNSSEC changesets are applied directly.
    575 int ret = apply_single(contents, ch, master);
    576 if (ret != KNOT_EOK) {
    577 update_cleanup(ch);
    578 return ret;
    524 579 }
    580
    581 ret = finalize_updated_zone(contents, true);
    • Author Contributor

      Shouldn't this be called by the caller rather than here? Then this function would become redundant at all and apply_single() may become an API function. Don't know, just thinking aloud here.

    • Author Contributor

      finalize_updated_zone is a static function, I don't think it should be made visible.

  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 53 65 *
    54 66 * \return KNOT_E*
    55 67 */
    56 int apply_changesets_directly(zone_contents_t *contents,
    57 changesets_t *chsets);
    68 int apply_changesets_directly(zone_contents_t *contents, list_t *chsets);
    69 int apply_changeset_directly(zone_contents_t *contents, changeset_t *ch);
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 53 65 *
    54 66 * \return KNOT_E*
    55 67 */
    56 int apply_changesets_directly(zone_contents_t *contents,
    57 changesets_t *chsets);
    68 int apply_changesets_directly(zone_contents_t *contents, list_t *chsets);
    69 int apply_changeset_directly(zone_contents_t *contents, changeset_t *ch);
    58 70
    59 71 /*!
    60 72 * \brief Cleanups successful update. (IXFR, DNSSEC, DDNS).
    61 73 * \param chgs Changesets used to create the update.
    62 74 */
    63 void update_cleanup(changesets_t *chgs);
    75 void updates_cleanup(list_t *chgs);
    76
    77 void update_cleanup(changeset_t *change);
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 63 void update_cleanup(changesets_t *chgs);
    75 void updates_cleanup(list_t *chgs);
    76
    77 void update_cleanup(changeset_t *change);
    64 78
    65 79 /*!
    66 80 * \brief Rollbacks failed update (IXFR, DNSSEC, DDNS).
    67 81 *
    68 * \param chgs Changesets used to create the update.
    69 * \param new_contents Created zone contents.
    82 * \param chgs Changesets used to create the update.
    70 83 */
    71 void update_rollback(changesets_t *chgs, zone_contents_t **new_contents);
    84 void updates_rollback(list_t *chgs);
    85
    86 void update_rollback(changeset_t *change);
  • Ghost User
    Ghost User @ghost started a thread on the diff
  • 71 if (ptrlist_add(&ch_it->iters, it, NULL) == NULL) {
    72 cleanup_iter_list(&ch_it->iters);
    73 return KNOT_ENOMEM;
    74 }
    75 }
    52 76 }
    53 77
    54 // Init list with changesets
    55 init_list(&chs->sets);
    78 va_end(args);
    56 79
    57 80 return KNOT_EOK;
    58 81 }
    59 82
    60 changesets_t *changesets_create(unsigned count)
    83 static void iter_next_node(changeset_iter_t *ch_it, hattrie_iter_t *t_it)
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading