On 27.01.2015 11:14, Daniel Shahaf wrote:
> [[[
> svn --version --verbose: Support /etc/os-release, the systemd "what distro
> am I running" API.
>
> * subversion/libsvn_subr/sysinfo.c
> (linux_release_name): Try /etc/os-release if /usr/bin/lsb_release fails.
> (systemd_release): New helper for linux_release_name().
> (remove_shell_quoting): New helper function.
> ]]]
>
> --- subversion/libsvn_subr/sysinfo.c
> +++ subversion/libsvn_subr/sysinfo.c
> @@ -410,6 +410,78 @@ lsb_release(apr_pool_t *pool)
> return NULL;
> }
>
> +/* Attempt to strip double quotes from a string.
> + *
> + * The string considered is (STR->DATA + OFFSET). If it starts
> + * and ends with double quotes, and has no escape sequences, return
> + * the string delimited by the double quotes. Otherwise, return
> + * the original string verbatim.
> + */
> +static const char *
> +remove_shell_quoting(svn_stringbuf_t *buf,
> + int offset,
> + apr_pool_t *result_pool)
> +{
> + const char *str = buf->data + offset;
> + const char *second_double_quote;
> +
> + /* Are there exactly two double quotes, at the first and last positions? */
> + if (str[0] == '"' && (second_double_quote = strchr(str+1, '"'))
> + && second_double_quote[1] == '\0')
Ouch. I don't like assignments in conditions; this bit can be rewritten
to be more readable. It's not as if we have to write the fastest
possible code here.
Also, the spec says that the values can be enclosed in single or double
quotes; but you only check for double quotes.
> + /* Are there any backslashes? */
> + if (!strchr(str, '\\'))
> + /* Return whatever is between the quotes. */
> + return apr_pstrndup(result_pool, str+1, second_double_quote - (str+1));
It should be fairly easy to strip the escapes from the string.
> @@ -527,6 +599,10 @@ linux_release_name(apr_pool_t *pool)
> Covers, for example, Debian, Ubuntu and SuSE. */
> const char *release_name = lsb_release(pool);
>
> + /* Try the systemd way (covers Arch). */
> + if (!release_name)
> + release_name = systemd_release(pool);
> +
> /* Try RHEL/Fedora/CentOS */
> if (!release_name)
> release_name = redhat_release(pool);
I'd try the redhat_release before systemd_release; for example, googling
lists reported bugs on CentOS, where /etc/os-release says "RHEL" but
/etc/redhat-releasesays "CentOS", so the latter is more likely to be
correct.
-- Brane
Received on 2015-01-27 11:42:46 CET