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

Re: svn commit: r18040 - in trunk/subversion: libsvn_delta libsvn_fs_fs libsvn_ra_dav libsvn_ra_svn libsvn_repos libsvn_subr libsvn_wc svn tests/libsvn_delta

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2006-01-17 01:34:05 CET

dlr@tigris.org writes:

> Author: dlr
> Date: Tue Jan 10 13:20:20 2006
> New Revision: 18040
>
> Modified:
> trunk/subversion/libsvn_delta/svndiff.c
> trunk/subversion/libsvn_fs_fs/fs_fs.c
> trunk/subversion/libsvn_fs_fs/revs-txns.c
> trunk/subversion/libsvn_ra_dav/session.c
> trunk/subversion/libsvn_ra_svn/client.c
> trunk/subversion/libsvn_repos/load.c
> trunk/subversion/libsvn_subr/config_file.c
> trunk/subversion/libsvn_wc/adm_files.c
> trunk/subversion/svn/lock-cmd.c
> trunk/subversion/tests/libsvn_delta/random-test.c
>
> Log:
> Cleanse GCC 4.0.1 compilation warnings.

Only recent gcc's that give these warnings, I think it's a combination
of '-funit-at-a-time' being enabled, which allows the optimiser to
look into the body of static function calls, together with the
optimiser not being aware that svn_error_create always returns
non-NULL. However humans know about svn_error_create and we can see
that the "may be used" never happens and so the initialisation is
unnecessary. The drawback of the unnecessary initialisation is that
it might make it harder for humans to understand the code and it
definitely defeats valgrind which can detect such uses if they get
tested.

It's possible to stop gcc reporting these warnings with
'-Wno-uninitialized' although that will also stop the warnings that
are not 'false alarms'. It's possible to change the optimiser with
'-fno-unit-at-a-time' but that might reduce the optimisation. I don't
know if we can tell the optimiser about the non-NULL return of
svn_error_create, but I think not.

Maybe we are stuck with these initialisations, but it does make me a
little uneasy. I realise that you want to get rid of the warnings, a
warning free build is a very good thing, but it's possible that the
cure is worse than the disease.

> * subversion/libsvn_wc/adm_files.c
> (svn_wc_ensure_adm2): Initialize local variable "exists_already" to
> FALSE.
>
> * subversion/libsvn_subr/config_file.c
> (svn_config__parse_file): Initialize "ctx.ungotten_char" to NUL.
>
> * subversion/tests/libsvn_delta/random-test.c
> (random_combine_test): Initialize local variable "seed" to 0.
>
> * subversion/libsvn_repos/load.c
> (svn_repos_parse_dumpstream2): Initialize local variable "version"
> to SVN_REPOS_DUMPFILE_FORMAT_VERSION,

Is SVN_REPOS_DUMPFILE_FORMAT_VERSION a good value? Since the
initialization is unnecessary the actual value doesn't matter at
present, but were the code to change then falsely claiming to have a
valid version might be a bad idea. Something "invalid" such as
INT_MIN or -1 might be better.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 17 01:45:06 2006

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