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

Re: [PATCH] Fix segfault when gnome keyring lookup fails

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Thu, 12 Jul 2018 18:59:50 +0000

Jan Palus wrote on Thu, 12 Jul 2018 20:23 +0200:
> On 12.07.2018 15:08, Daniel Shahaf wrote:
> > Thanks for the patch; however, I don't think that's the correct fix.
>
> To be honest I wasn't sure of that either but on the other hand I thought that
> having initialized state won't hurt.
>

Initializing the variable prevents the compiler from issueing "This
variable is used uninitialized" if some codepath uses that variable
before calling password_get() to initialize it. We tend to prefer
leaving variables uninitialized for this reason.

> > Does the following patch fix the problem? Speaking of which, could you explain
> > how to reproduce the problem?
>
> As expected the patch does fix the issue, however the question about reproducer
> is somewhat problematic. No matter what I try I cannot reproduce it when
> invoking subversion manually from command line (ie with empty environment). The
> problem manifests itself only when using automation script which involves
> gradle->svnant->native subversion binary. I've added basic debug statements and
> it appears following condition fails already:
...
> The main difference between invoking svn directly and through the script is
> value of uninitialized "done". In script it is always non-zero while invoked
> directly it's the opposite. If you manually force done to some random, non-zero
> value the reproducer goes down to:
>
> env -i svn co --no-auth-cache <url> --username user --password pass --non-interactive

Thanks. I'm not sure sure what's causing the observed difference in the
behaviour of the local variable 'done', but in any case the compiler is
within its rights to leave that variable uninitialized, so I've gone
ahead and committed the patch --- r1835760 --- and proposed it for
backport to to 1.10.x. It should be backported in time for 1.10.2
(possibly 1.10.1 but no guarantees).

Cheers,

Daniel
Received on 2018-07-12 21:00:01 CEST

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