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

enums , ints, and authz

From: Paul Burba <paulb_at_softlanding.com>
Date: 2006-01-18 16:50:25 CET

Hello All,

Note: This is *not* an iSeries specific question, so please don't let your
eyes glaze over at the start :-)

While working on our ebcdic port of subversion to the IBM iSeries we
experienced an unusual(?) problem:

When compiling a C program on the iSeries there is an option to set the
size of enums to 1 byte, 2 bytes, 4 bytes, the ANSI-standard for ints (4
bytes), or the smallest number of bytes that can contain the enum. The
last option is the default and what we used. We had no problems when
building subversion with no optimization, but when we built with full
optimization, authz started behaving inconsistently, denying access and
granting access with seemingly no relation the authz file being used.
After much painful debugging I was able to track the problem down to
libsvn/repos/authz.c's authz_access_is_granted() and
authz_access_is_determined():

static svn_boolean_t
authz_access_is_granted (svn_repos_authz_access_t allow,
                         svn_repos_authz_access_t deny,
                         svn_repos_authz_access_t required)
{
  svn_repos_authz_access_t stripped_req =
    required & (svn_authz_read | svn_authz_write);

  if ((deny & required) == svn_authz_none)
    return TRUE;
  else if ((allow & required) == stripped_req)
    return TRUE;
  else
    return FALSE;
}

static svn_boolean_t
authz_access_is_determined (svn_repos_authz_access_t allow,
                            svn_repos_authz_access_t deny,
                            svn_repos_authz_access_t required)
{
  if ((deny & required) || (allow & required))
    return TRUE;
  else
    return FALSE;
}

The optimized code didn't perform the bitwise operations in these
functions correctly when the enums were stored in the smallest possible
byte size. What is interesting is that we had no problems in 1.2 using
the same authz file. In 1.2 the functionality of
authz_access_is_granted() and authz_access_is_determined() was in
mod_dav_authz.c and was directly in the functions parse_authz_lines() and
parse_authz_section(), e.g.:

static int parse_authz_lines(svn_config_t *cfg,
                             const char *repos_name, const char
*repos_path,
                             const char *user,
                             int required_access, int *granted_access,
                             apr_pool_t *pool)
{
    const char *qualified_repos_path;
    struct parse_authz_baton baton = { 0 };

    baton.pool = pool;
    baton.config = cfg;
    baton.user = user;

    /* First try repos specific */
    qualified_repos_path = apr_pstrcat(pool, repos_name, ":", repos_path,
                                       NULL);
    svn_config_enumerate(cfg, qualified_repos_path,
                         parse_authz_line, &baton);
    *granted_access = !(baton.deny & required_access)
                      || (baton.allow & required_access);

    if ((baton.deny & required_access)
        || (baton.allow & required_access))
        return TRUE;

    svn_config_enumerate(cfg, repos_path,
                         parse_authz_line, &baton);
    *granted_access = !(baton.deny & required_access)
                      || (baton.allow & required_access);

    return (baton.deny & required_access)
           || (baton.allow & required_access);
}

Back to our 1.3 problem, I found that by casting the arguments in authz.c
to ints the problem went away, e.g.:

static svn_boolean_t
authz_access_is_determined (svn_repos_authz_access_t allow,
                            svn_repos_authz_access_t deny,
                            svn_repos_authz_access_t required)
{
  if (((int)deny & (int)required) || ((int)allow & (int)required))
    return TRUE;
  else
    return FALSE;
}

Removing these casts, we rebuilt our fully optimized code using the option
to always make enums 4 bytes and the authz problems went away. My best
guess is that 1.2 had no problems because the AUTHZ_SVN_* enums were
always implicitly cast to an int when used. In 1.3 however, with the
creation of authz_access_is_granted() and authz_access_is_determined(),
ints (authz_lookup_baton->allow, deny, required_access) were passed to
functions that take 1-byte enums and something went awry.

Anyway, this gets (finally!) to my ultimate questions:

1) Could this problem afflict other platforms that have compile options to
dictate enum size? (e.g. gcc's -fshort-enums option)

2) In C is it always safe to pass an int argument to a function that takes
an enum argument? The ever-trusty Harbinson and Steele C Reference Manual
*seems* to indicate this is fine for compilers that strictly implement the
standard, but then warns to not do it:

"...enumerated data types in Standard C...act as little more than slightly
readable ways to name integer constants. As a matter of style, we suggest
that programmers treat enumerated types as different from integers and not
mix them in integer expressions without using casts. In fact, some UNIX C
compilers implement a weakly typed form of enumerations in which some
conversions between enumerated types and integers are not permitted
without casts."

Any thoughts from the C gurus out there are appreciated.

Thanks,

Paul B.

_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 18 20:29:27 2006

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