On Monday, November 14, 2011 8:27 AM, "NormW" <no...@gknw.net> wrote:
> Hi,
> Aimed the Open Watcom Compiler at 1.7.1 subversion source and was
> pleasantly surprised how much built on the first go.
>
> The one real blip was that OWC 1.9 (nor a near future release 2.0)
> supports 64-bit switch expressions, and there is one such used in:
> .\subversion\libsvn_ra_svn\client.c (1170).
>
> While annoying, a change similar to the following should suit all likely
> compilers in this case:
>
> > --- client.c.orig 2011-09-28 02:30:36.000000000 +1000
> > +++ client.c 2011-11-13 06:14:09.109375000 +1100
> > @@ -1166,15 +1166,12 @@
> > static svn_tristate_t
> > optbool_to_tristate(apr_uint64_t v)
> > {
> > - switch (v)
> > - {
> > - case TRUE:
> > - return svn_tristate_true;
> > - case FALSE:
> > - return svn_tristate_false;
> > - default: /* Contains SVN_RA_SVN_UNSPECIFIED_NUMBER */
> > - return svn_tristate_unknown;
> > - }
> > + if (v == TRUE)
> > + return svn_tristate_true;
> > + if (v == FALSE)
> > + return svn_tristate_false;
> > +
> > + return svn_tristate_unknown; /* Contains SVN_RA_SVN_UNSPECIFIED_NUMBER */
> > }
>
Well, perhaps fix the compiler instead of rewriting everyone's switch
statements? :-)
(But not opposed to committing this if people can't build svn
because of this.)
> FYI: The 'warn' blips; at least the first 2 are already noted in the
> source, while for #3 it's unclear how this can return an int....
>
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_delta\compose_delta.c(707):
> > Warning! W124: Comparison result always 1
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_repos\reporter.c(189):
> > Warning! W124: Comparison result always 0
As you say: known issues noted in comments. Yes, would be nice to fix
them someday.
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_subr\pool.c(55):
> > Warning! W107: Missing return value for function 'abort_on_pool_failure'
If we make this return an int then we'd have to cast it at the callsite
(it's a callback function). *shrug*
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_subr\skel.c(233):
> > Warning! W136: Comparison equivalent to 'unsigned == 0'
The code is correct (and future-proof if len ever becomes signed).
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_subr\stream.c(1066):
> > Warning! W136: Comparison equivalent to 'unsigned == 0'
Same
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_ra_svn\marshal.c(577):
> > Warning! W124: Comparison result always 0
Same issue as in the 2nd warning: checks for overflow by checking
whether it's "> APR_SIZE_MAX". Perhaps we should restructure the check?
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_ra_neon\merge.c(159):
> > Warning! W113: Pointer type mismatch
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_ra_neon\merge.c(159):
> > Note! N2003: source conversion type is 'void *'
> > D:\Projects\srcs\subversion-1.7.1\subversion\libsvn_ra_neon\merge.c(159):
> > Note! N2004: target conversion type is 'enum svn_recurse_kind '
Casting an enum { x=1, y=2 } to/from a void*.
We could restructure this with sentinels, I suppose:
static void *recurse[2];
apr_hash_set(valid_targets, key, klen, &recurse[FALSE]);
apr_hash_set(valid_targets, key, klen, &recurse[TRUE]);
> > D:\Projects\srcs\subversion-1.7.1\subversion\svnlook\main.c(2376): Warning!
> > W136: Comparison equivalent to 'unsigned == 0'
Again: too trigger-happy. It's valid to ask whether an unsigned is nonpositive.
>
> Not saying these are bugs - just what the OWC 1.9 says. (Hence FYI).
> Norm
>
Thanks for your input,
Daniel
