Re: advisory locks patch

[Mark D. Baushke]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Matthew,

C99 compilers will likely have problems with a few parts of your code as
written... For example, gcc gives me the following problems with edit.c:

../../src/edit.c: In function `check_fileproc':
../../src/edit.c:356: warning: passing arg 2 of `send_file_names' from 
incompatible pointer type
../../src/edit.c: In function `unedit':
../../src/edit.c:804: syntax error before `char'
../../src/edit.c:837: `commandName' undeclared (first use in this function)
../../src/edit.c:837: (Each undeclared identifier is reported only once
../../src/edit.c:837: for each function it appears in.)

On line 356, you are passing &finfo->fullname which is a 'const char **'
to a function that wants a 'char **' and these two types are
incompatible.

Something like this will fix it...

#ifdef CLIENT_SUPPORT
    if (current_parsed_root->isremote)
    {
        char *fullname = xstrdup (finfo->fullname);
        send_file_names (1, &fullname, SEND_EXPAND_WILD);
        if (fullname)
            free (fullname);
    }
    else
#endif /* CLIENT_SUPPORT */

because if provides temporary storage that the send_file_names function
could hack if it wished (not that it does at present... possibly the
send_file_names() function should take a 'const char**argv' argument
instead...

The problem on line 804 (and 837) is that C99 does not allow you to have
declarations after you have had executable code in the current scope.

... This set of lines ...
unedit (int argc, char **argv)
{
    int local = 0;
    int c;
    int err;
    int startAndShutdown=1;

    if (argc == -1)
        usage (unedit_usage);

    char *commandName = argv[0];

... needs to be altered to something like this ...

int
unedit (int argc, char **argv)
{
    int local = 0;
    int c;
    int err;
    int startAndShutdown=1;
    char *commandName;

    if (argc == -1)
        usage (unedit_usage);

    commandName = argv[0];

... in order to compile correctly.

For myself, I would find it desirable for someone to address what should
happen if userA creates an advisory lock on moduleA/fileA.c on the
mainline and userB tries to create an advisory lock on moduleA/fileA.c
on some FOO_BRANCH branch of the mainline... the documentation seems to
be silent on what should happen...

For what it is worth, if a 'development team' believes that advisory
locks should be enabled, I suggest that adding a new CVSROOT/config
option to change the default for the repository and/or some list of
modules within the repository may be a more desirable alternative than
trusting that the users are all using a .cvsrc switch for the purpose.
Especially as they may have more than one repository in use that may
have different policies.

I'll let Derek determine what parts of the patch should be committed to
the FEATURE branch.

        Thanks,
        -- Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFBMEEl3x41pRYZE/gRArugAJ4yexayAs+7ZS3igoGXkj+ti94NUwCgwd2p
EmG3ZOnWGQsWh1Uez4ZdWRY=
=wQ4x
-----END PGP SIGNATURE-----