bc28f7693d095758275448410aaa91255482857a originally added this, but the implementation isn't correct and causes excessive notifications when writing to other options which may have bad behavior in some some circumstances. Fix this by reworking the implementation for force options so that the timestamps of the option update compared instead. Whenever an option gets changed, the internal timestamp in the cache is always incremented. For a special force option, we can save this timestamp internally as well. Because cache_check_update is always checked before potentially sending an option notification, we know that if the internal timestamp is equal to the timestamp saved by a force update option, it must have been previously written. Thus, the notification can be sent. This lets options like geometry work repeatedly but without constantly sending notifications. Fixes #13954.
Unified · Split
| @@ -99,12 +99,17 @@ struct config_cache { · 98 unmodified lines | ||
| 99 | 99 | void *wakeup_cb_ctx; |
| 100 | 100 | }; |
| 101 | 101 | |
| 102 | +struct force_update { | |
| 103 | + char *name; | |
| 104 | + uint64_t ts; | |
| 105 | +}; | |
| 106 | + | |
| 102 | 107 | // Per m_config_data state for each m_config_group. |
| 103 | 108 | struct m_group_data { |
| 104 | − char *udata; // pointer to group user option struct | |
| 105 | − uint64_t ts; // timestamp of the data copy | |
| 106 | − char **force_updates; // track if any opt in group was written with force_update | |
| 107 | − int force_updates_len; | |
| 109 | + char *udata; // pointer to group user option struct | |
| 110 | + uint64_t ts; // timestamp of the data copy | |
| 111 | + struct force_update **force_update; // tracks opts that are written with force update | |
| 112 | + int force_update_len; | |
| 108 | 113 | }; |
| 109 | 114 | |
| 110 | 115 | static void add_sub_group(struct m_config_shadow *shadow, const char *name_prefix, |
| @@ -590,23 +595,30 @@ struct m_config_cache *m_config_cache_alloc(void *ta_parent, · 479 unmodified lines | ||
| 590 | 595 | return m_config_cache_from_shadow(ta_parent, global->config, group); |
| 591 | 596 | } |
| 592 | 597 | |
| 593 | −static void clear_force_update_list(struct m_group_data *gsrc) | |
| 598 | +static void append_force_update(struct m_config_cache *cache, struct m_group_data *gdata, | |
| 599 | + const char *opt_name) | |
| 594 | 600 | { |
| 595 | − int index = 0; | |
| 596 | − while (index < gsrc->force_updates_len) { | |
| 597 | − TA_FREEP(&gsrc->force_updates[index]); | |
| 598 | − ++index; | |
| 601 | + for (int i = 0; i < gdata->force_update_len; ++i) { | |
| 602 | + if (strcmp(opt_name, gdata->force_update[i]->name) == 0) { | |
| 603 | + gdata->force_update[i]->ts = gdata->ts; | |
| 604 | + return; | |
| 605 | + } | |
| 599 | 606 | } |
| 600 | − gsrc->force_updates_len = 0; | |
| 607 | + struct force_update *new_update = talloc_zero(cache, struct force_update); | |
| 608 | + new_update->name = talloc_strdup(cache, opt_name); | |
| 609 | + new_update->ts = gdata->ts; | |
| 610 | + MP_TARRAY_APPEND(cache, gdata->force_update, gdata->force_update_len, new_update); | |
| 601 | 611 | } |
| 602 | 612 | |
| 603 | −static bool check_force_update_list(struct m_group_data *gsrc, const char *opt_name) | |
| 613 | +static bool check_force_update(struct m_group_data *gdata, const char *opt_name, | |
| 614 | + uint64_t timestamp) | |
| 604 | 615 | { |
| 605 | − int index = 0; | |
| 606 | − while (index < gsrc->force_updates_len) { | |
| 607 | − if (strcmp(opt_name, gsrc->force_updates[index]) == 0) | |
| 616 | + for (int i = 0; i < gdata->force_update_len; ++i) { | |
| 617 | + if ((strcmp(opt_name, gdata->force_update[i]->name) == 0) && | |
| 618 | + gdata->force_update[i]->ts == timestamp) | |
| 619 | + { | |
| 608 | 620 | return true; |
| 609 | − ++index; | |
| 621 | + } | |
| 610 | 622 | } |
| 611 | 623 | return false; |
| 612 | 624 | } |
| @@ -637,8 +649,8 @@ static void update_next_option(struct m_config_cache *cache, void **p_opt) · 24 unmodified lines | ||
| 637 | 649 | |
| 638 | 650 | if (opt->offset >= 0 && opt->type->size) { |
| 639 | 651 | bool opt_equal = m_option_equal(opt, ddst, dsrc); |
| 640 | − bool force_update = opt->force_update && gsrc->force_updates_len && | |
| 641 | − check_force_update_list(gsrc, opt->name); | |
| 652 | + bool force_update = opt->force_update && | |
| 653 | + check_force_update(gsrc, opt->name, in->ts); | |
| 642 | 654 | if (!opt_equal || force_update) { |
| 643 | 655 | uint64_t ch = get_opt_change_mask(dst->shadow, |
| 644 | 656 | in->upd_group, dst->group_index, opt); |
| @@ -666,8 +678,6 @@ static void update_next_option(struct m_config_cache *cache, void **p_opt) · 21 unmodified lines | ||
| 666 | 678 | } |
| 667 | 679 | |
| 668 | 680 | gdst->ts = gsrc->ts; |
| 669 | − } else { | |
| 670 | − clear_force_update_list(gsrc); | |
| 671 | 681 | } |
| 672 | 682 | |
| 673 | 683 | in->upd_group++; |
| @@ -791,10 +801,8 @@ bool m_config_cache_write_opt(struct m_config_cache *cache, void *ptr) · 117 unmodified lines | ||
| 791 | 801 | } |
| 792 | 802 | } |
| 793 | 803 | |
| 794 | − if (opt->force_update) { | |
| 795 | − MP_TARRAY_APPEND(shadow, gsrc->force_updates, gsrc->force_updates_len, | |
| 796 | − talloc_strdup(shadow, opt->name)); | |
| 797 | − } | |
| 804 | + if (opt->force_update) | |
| 805 | + append_force_update(cache, gsrc, opt->name); | |
| 798 | 806 | |
| 799 | 807 | mp_mutex_unlock(&shadow->lock); |
| 800 | 808 | |