[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svn --version --verbose += /etc/os-release

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Tue, 27 Jan 2015 11:32:25 +0000

Branko Čibej wrote on Tue, Jan 27, 2015 at 11:41:27 +0100:
> On 27.01.2015 11:14, Daniel Shahaf wrote:
> > + /* 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.
>

I wasn't optimizing for speed; I thought this was the most
straightforward way to implement the condition described in the
associated comment. But this will need to change anyway when \ parsing
is added.

> > + /* 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.
>

Yes, see in my reply to Stefan.

> > @@ -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-release says "CentOS", so the latter is more likely to be
> correct.
>

I would call this a vendor bug. Debian has a similar issue, where
/etc/os-release isn't as specific as lsb_release:

    % head -n1 /etc/os-release
    PRETTY_NAME="Debian GNU/Linux 7 (wheezy)"
    % lsb_release -d
    Description: Debian GNU/Linux 7.8 (wheezy)

Having said that, I don't have a strong preference about the order.
Whatever people decide is fine by me.

Thanks,

Daniel

> -- Brane
Received on 2015-01-27 12:32:57 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.