Re: OWC meets subversion.

On Monday, November 14, 2011 8:27 AM, "NormW" <normw_at_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
Received on 2011-11-14 07:27:17 CET