[gnutls-devel] GnuTLS | priority: rework config reloading logic and locking (!1483)

[Read-only notification of GnuTLS library development activities]

Merge request https://gitlab.com/gnutls/gnutls/-/merge_requests/1483 was reviewed by Alexander Sosedkin

--
  
Alexander Sosedkin started a new discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_722674749

> +	/* Another thread has updated the system wide config while upgrading to
> +	 * write lock. */
> +	if (system_priority_last_mod >= sb.st_mtime) {

Previously we haven't depended on mtime monotonicity, now we do. Moving a file or restoring from a backup could be changes that jump back in mtime that we would better still pick up.

--
  
Alexander Sosedkin started a new discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_722674752

> +	/* Another thread has updated the system wide config while upgrading to
> +	 * write lock. */
> +	if (system_priority_last_mod >= sb.st_mtime) {

Here we implicitly use `system_priority_last_mod` as a flag of the config being loaded. I'd either check `system_priority_file_loaded` as well for clarity or drop `system_priority_file_loaded` and leave just `system_priority_last_mod` if it's possible and convenient.

--
  
Alexander Sosedkin started a new discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_722674753

> +	}
> +		
> +	_name_val_array_clear(&system_wide_config.priority_strings);

Minor: I'd clear `system_priority_file_loaded` even if there are no immediate benefits.

--
  
Alexander Sosedkin started a new discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_722674754

>   *
> - * Returns: a constant pointer to the config file loaded, or %NULL if none
> + * Returns: a constant pointer to the config file

I'd extend to `file path`, maybe.

--
  
Alexander Sosedkin started a new discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_722674758

>  
> +	/* This requires a lock so there are no inconsistencies in system_wide_*
> +	 * loaded from the config file. */

Nit: not updated since `system_wide_config` grouping has happened.

--
  
Alexander Sosedkin started a new discussion on lib/priority.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483#note_722674768

> - * file loaded by the library. The returned pointer is valid
> - * until the library is unloaded.
> + * file to be loaded by the library.

Why remove this sentence? That'd restrict our potential future maneuvers, while the current limitation is sensible.


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/1483
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20211103/d8bf06dd/attachment.html>