[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Cvs-cvs] ccvs/src ChangeLog lock.c sanity.sh
From: |
Derek Robert Price |
Subject: |
[Cvs-cvs] ccvs/src ChangeLog lock.c sanity.sh |
Date: |
Fri, 19 Sep 2008 00:48:09 +0000 |
CVSROOT: /cvsroot/cvs
Module name: ccvs
Changes by: Derek Robert Price <dprice> 08/09/19 00:48:09
Modified files:
src : ChangeLog lock.c sanity.sh
Log message:
* lock.c: Some cleanup.
(lock_obtained, lock_wait): Only print message when !really_quiet.
(set_lock): Don't print error messages, but return errno and...
(Reader_Lock, set_promotable_lock, lock_dir_for_write, internal_lock):
...rely on callers to report the error.
(set_readlock_name, readers_exist): Declare inline.
(multiroot3-10): Update to compensate for new error messages.
CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/ccvs/src/ChangeLog?cvsroot=cvs&r1=1.3599&r2=1.3600
http://cvs.savannah.gnu.org/viewcvs/ccvs/src/lock.c?cvsroot=cvs&r1=1.129&r2=1.130
http://cvs.savannah.gnu.org/viewcvs/ccvs/src/sanity.sh?cvsroot=cvs&r1=1.1204&r2=1.1205
Patches:
Index: ChangeLog
===================================================================
RCS file: /cvsroot/cvs/ccvs/src/ChangeLog,v
retrieving revision 1.3599
retrieving revision 1.3600
diff -u -b -r1.3599 -r1.3600
--- ChangeLog 18 Sep 2008 17:43:55 -0000 1.3599
+++ ChangeLog 19 Sep 2008 00:48:05 -0000 1.3600
@@ -1,5 +1,13 @@
2008-09-18 Derek R. Price <address@hidden>
+ * lock.c: Some cleanup.
+ (lock_obtained, lock_wait): Only print message when !really_quiet.
+ (set_lock): Don't print error messages, but return errno and...
+ (Reader_Lock, set_promotable_lock, lock_dir_for_write, internal_lock):
+ ...rely on callers to report the error.
+ (set_readlock_name, readers_exist): Declare inline.
+ (multiroot3-10): Update to compensate for new error messages.
+
* client.c, commit.c, fileattr.c, find_names.c: Some cleanup.
* hash.c (walklist): Make it OK for callback procs to delete the node
Index: lock.c
===================================================================
RCS file: /cvsroot/cvs/ccvs/src/lock.c,v
retrieving revision 1.129
retrieving revision 1.130
diff -u -b -r1.129 -r1.130
--- lock.c 17 Sep 2008 19:53:30 -0000 1.129
+++ lock.c 19 Sep 2008 00:48:07 -0000 1.130
@@ -74,9 +74,12 @@
# include <config.h>
#endif
-/* Verify API */
+/* Validate API. */
#include "lock.h"
+/* GNULIB */
+#include "quote.h"
+
/* CVS */
#include "parseinfo.h"
#include "recurse.h"
@@ -170,11 +173,12 @@
/* Return a newly malloc'd string containing the name of the lock for the
- repository REPOSITORY and the lock file name within that directory
- NAME. Also create the directories in which to put the lock file
- if needed (if we need to, could save system call(s) by doing
- that only if the actual operation fails. But for now we'll keep
- things simple). */
+ * repository REPOSITORY and the lock file name within that directory
+ * NAME. Also create the directories in which to put the lock file
+ * if needed (if we need to, could save system call(s) by doing
+ * that only if the actual operation fails. But for now we'll keep
+ * things simple).
+ */
static char *
lock_name (const char *repository, const char *name)
{
@@ -185,15 +189,15 @@
int saved_umask = 0;
TRACE (TRACE_FLOW, "lock_name (%s, %s)",
- repository ? repository : "(null)", name ? name : "(null)");
+ TRACE_NULL (repository), TRACE_NULL (name));
if (!config->lock_dir)
{
/* This is the easy case. Because the lock files go directly
in the repository, no need to create directories or anything. */
- assert (name != NULL);
- assert (repository != NULL);
- retval = Xasprintf ("%s/%s", repository, name);
+ assert (name);
+ assert (repository);
+ retval = dir_append (repository, name);
}
else
{
@@ -241,20 +245,21 @@
}
/* Now add the directories one at a time, so we can create
- them if needed.
-
- The idea behind the new_mode stuff is that the directory we
- end up creating will inherit permissions from its parent
- directory (we re-set new_mode with each EEXIST). CVSUMASK
- isn't right, because typically the reason for LockDir is to
- use a different set of permissions. We probably want to
- inherit group ownership also (but we don't try to deal with
- that, some systems do it for us either always or when g+s is on).
-
- We don't try to do anything about the permissions on the lock
- files themselves. The permissions don't really matter so much
- because the locks will generally be removed by the process
- which created them. */
+ * them if needed.
+ *
+ * The idea behind the new_mode stuff is that the directory we
+ * end up creating will inherit permissions from its parent
+ * directory (we re-set new_mode with each EEXIST). CVSUMASK
+ * isn't right, because typically the reason for LockDir is to
+ * use a different set of permissions. We probably want to
+ * inherit group ownership also (but we don't try to deal with
+ * that, some systems do it for us either always or when g+s is on).
+ *
+ * We don't try to do anything about the permissions on the lock
+ * files themselves. The permissions don't really matter so much
+ * because the locks will generally be removed by the process
+ * which created them.
+ */
if (stat (config->lock_dir, &sb) < 0)
error (1, errno, "cannot stat %s", config->lock_dir);
@@ -283,7 +288,7 @@
*
* INPUTS
* lock The lock to remove.
- * free True if this lock directory will not be reused (free
+ * free_repository True if this lock directory will not be reused (free
* lock->repository if necessary).
*/
static void
@@ -468,19 +473,17 @@
/*
* Set the global readlock variable if it isn't already.
*/
-static void
+static inline void
set_readlock_name (void)
{
- if (readlock == NULL)
- {
+ if (!readlock)
readlock = Xasprintf (
#ifdef HAVE_LONG_FILE_NAMES
"%s.%s.%ld", CVSRFL, hostname,
#else
"%s.%ld", CVSRFL,
#endif
- (long) getpid ());
- }
+ (long) getpid());
}
@@ -491,11 +494,13 @@
static void
lock_obtained (const char *repos)
{
+ if (!really_quiet)
+ {
time_t now;
char *msg;
struct tm *tm_p;
- (void) time (&now);
+ time (&now);
tm_p = gmtime (&now);
msg = Xasprintf ("[%8.8s] obtained lock in %s",
(tm_p ? asctime (tm_p) : ctime (&now)) + 11, repos);
@@ -504,6 +509,7 @@
soon as possible. */
cvs_flusherr ();
free (msg);
+ }
}
@@ -514,11 +520,13 @@
static void
lock_wait (const char *repos)
{
+ if (!really_quiet)
+ {
time_t now;
char *msg;
struct tm *tm_p;
- (void) time (&now);
+ time (&now);
tm_p = gmtime (&now);
msg = Xasprintf ("[%8.8s] waiting for %s's lock in %s",
(tm_p ? asctime (tm_p) : ctime (&now)) + 11,
@@ -528,7 +536,9 @@
soon as possible. */
cvs_flusherr ();
free (msg);
- (void)sleep (CVSLCKSLEEP);
+ }
+
+ sleep (CVSLCKSLEEP);
}
@@ -542,6 +552,23 @@
* seconds old, just try to remove the directory.
* #endif
*
+ * INPUTS
+ * lock Specification of lock to set.
+ * will_wait Whether to wait for the lock or to return immediately when it
+ * is held by another process.
+ *
+ * GLOBALS
+ * errno Set by system calls on failure.
+ *
+ * ERRORS
+ * On unrecoverable failures to create or stat the lock dir, L_ERROR is
+ * returned and the global ERRNO will be set from the failed system call.
+ *
+ * RETURNS
+ * L_OK When the lock has been obtained.
+ * L_ERROR On unrecoverable errors. Gloabl ERRNO will be set.
+ * L_LOCKED When WILL_WAIT is set, this may be returned when another
+ * process already holds a lock.
*/
static int
set_lock (struct lock *lock, bool will_wait)
@@ -572,40 +599,37 @@
{
status = -1;
omask = umask (cvsumask);
- SIG_beginCrSect ();
+ SIG_beginCrSect();
if (CVS_MKDIR (masterlock, 0777) == 0)
{
lock->lockdir = masterlock;
- SIG_endCrSect ();
+ SIG_endCrSect();
status = L_OK;
if (waited)
lock_obtained (lock->repository);
goto after_sig_unblock;
}
- SIG_endCrSect ();
+ SIG_endCrSect();
after_sig_unblock:
- (void) umask (omask);
+ umask (omask);
if (status != -1)
goto done;
if (errno != EEXIST)
{
- error (0, errno,
- "failed to create lock directory for `%s' (%s)",
- lock->repository, masterlock);
status = L_ERROR;
goto done;
}
/* Find out who owns the lock. If the lock directory is
- non-existent, re-try the loop since someone probably just
- removed it (thus releasing the lock). */
+ * non-existent, re-try the loop since someone probably just
+ * removed it (thus releasing the lock).
+ */
if (stat (masterlock, &sb) < 0)
{
if (existence_error (errno))
continue;
- error (0, errno, "couldn't stat lock directory `%s'", masterlock);
status = L_ERROR;
goto done;
}
@@ -616,7 +640,7 @@
* ago, try to clean-up the lock directory, and if successful, just
* quietly retry to make it.
*/
- (void) time (&now);
+ time (&now);
if (now >= (sb.st_ctime + CVSLCKAGE))
{
if (CVS_RMDIR (masterlock) >= 0)
@@ -642,7 +666,7 @@
struct timespec ts;
ts.tv_sec = 0;
ts.tv_nsec = us * 1000;
- (void)nanosleep (&ts, NULL);
+ nanosleep (&ts, NULL);
continue;
}
}
@@ -668,19 +692,15 @@
int err = 0;
FILE *fp;
+ /* we only do one directory at a time for read locks! */
+ assert (!global_readlock.repository);
+
TRACE (TRACE_FUNCTION, "Reader_Lock (%s)", xrepository);
if (noexec || readonlyfs)
return 0;
- /* we only do one directory at a time for read locks! */
- if (global_readlock.repository != NULL)
- {
- error (0, 0, "Reader_Lock called while read locks set - Help!");
- return 1;
- }
-
- set_readlock_name ();
+ set_readlock_name();
/* remember what we're locking (for Lock_Cleanup) */
global_readlock.repository = xstrdup (xrepository);
@@ -688,29 +708,28 @@
/* get the lock dir for our own */
if (set_lock (&global_readlock, true) != L_OK)
- {
- error (0, 0, "failed to obtain dir lock in repository `%s'",
- xrepository);
- if (readlock != NULL)
- free (readlock);
- readlock = NULL;
- /* We don't set global_readlock.repository to NULL. I think this
- only works because recurse.c will give a fatal error if we return
- a nonzero value. */
- return 1;
- }
+ goto error;
/* write a read-lock */
global_readlock.file1 = lock_name (xrepository, readlock);
- if ((fp = CVS_FOPEN (global_readlock.file1, "w+")) == NULL
+ if (!(fp = CVS_FOPEN (global_readlock.file1, "w+"))
|| fclose (fp) == EOF)
- {
- error (0, errno, "cannot create read lock in repository `%s'",
- xrepository);
+ goto error;
+
+ goto done;
+
+error:
err = 1;
- }
+ if (!really_quiet)
+ error (0, errno, "failed to obtain read lock in %s",
+ quote (xrepository));
- /* free the lock dir */
+done:
+ if (err)
+ /* Clean up any and all locks. */
+ remove_lock_files (&global_readlock, true);
+ else
+ /* Just clear the lock dir, leaving the new read lock in place. */
clear_lock (&global_readlock);
return err;
@@ -750,21 +769,18 @@
int ret;
#ifdef CVS_FUDGELOCKS
time_t now;
- (void)time (&now);
+ time (&now);
#endif
TRACE (TRACE_FLOW, "lock_exists (%s, %s, %s)",
- repository, filepat, ignore ? ignore : "(null)");
+ repository, filepat, TRACE_NULL (ignore));
lockdir = lock_name (repository, "");
-
- assert (lockdir != NULL);
-
- lockdir[strlen (lockdir) - 1] = '\0'; /* remove trailing slash */
+ assert (lockdir);
do {
- if ((dirp = CVS_OPENDIR (lockdir)) == NULL)
- error (1, 0, "cannot open directory %s", lockdir);
+ if (!(dirp = CVS_OPENDIR (lockdir)))
+ error (1, 0, "cannot open directory %s", quote (lockdir));
ret = 0;
errno = 0;
@@ -834,7 +850,7 @@
*
* See lock_exists() for argument detail.
*/
-static int
+static inline int
readers_exist (const char *repository)
{
TRACE (TRACE_FLOW, "readers_exist (%s)", repository);
@@ -879,8 +895,15 @@
/*
- * Create a lock file for potential writers returns L_OK if lock set ok,
- * L_LOCKED if lock held by someone else or L_ERROR if an error occurred.
+ * Create a lock file for potential writers.
+ *
+ * ERRORS
+ * Prints a message before L_ERROR is returned.
+ *
+ * RETURNS
+ * L_OK Got the requested LOCK.
+ * L_LOCKED Didn't get the LOCK because another process holds it.
+ * L_ERROR Didn't obtain the LOCK because of a non-recoverable error.
*/
static int
set_promotable_lock (struct lock *lock)
@@ -889,9 +912,9 @@
FILE *fp;
TRACE (TRACE_FUNCTION, "set_promotable_lock(%s)",
- lock->repository ? lock->repository : "(null)");
+ TRACE_NULL (lock->repository));
- if (promotablelock == NULL)
+ if (!promotablelock)
{
promotablelock = Xasprintf (
#ifdef HAVE_LONG_FILE_NAMES
@@ -918,20 +941,21 @@
/* write the promotable-lock file */
lock->file1 = lock_name (lock->repository, promotablelock);
- if ((fp = CVS_FOPEN (lock->file1, "w+")) == NULL || fclose (fp) == EOF)
+ if (!(fp = CVS_FOPEN (lock->file1, "w+")) || fclose (fp) == EOF)
{
int xerrno = errno;
- if (CVS_UNLINK (lock->file1) < 0 && ! existence_error (errno))
- error (0, errno, "failed to remove lock %s", lock->file1);
+ if (CVS_UNLINK (lock->file1) < 0 && !existence_error (errno))
+ error (0, errno, "failed to remove lock %s",
+ quote (lock->file1));
/* free the lock dir */
clear_lock (lock);
/* return the error */
error (0, xerrno,
- "cannot create promotable lock in repository `%s'",
- lock->repository);
+ "cannot create promotable lock in repository %s",
+ quote (lock->repository));
return L_ERROR;
}
@@ -941,14 +965,15 @@
* decided that versions of CVS earlier than 1.12.4 are not likely to
* be used, this code can be removed.
*/
- set_readlock_name ();
+ set_readlock_name();
lock->file2 = lock_name (lock->repository, readlock);
- if ((fp = CVS_FOPEN (lock->file2, "w+")) == NULL || fclose (fp) == EOF)
+ if (!(fp = CVS_FOPEN (lock->file2, "w+")) || fclose (fp) == EOF)
{
int xerrno = errno;
- if ( CVS_UNLINK (lock->file2) < 0 && ! existence_error (errno))
- error (0, errno, "failed to remove lock %s", lock->file2);
+ if (CVS_UNLINK (lock->file2) < 0 && !existence_error (errno))
+ error (0, errno, "failed to remove lock %s",
+ quote (lock->file2));
/* free the lock dir */
clear_lock (lock);
@@ -959,8 +984,8 @@
/* return the error */
error (0, xerrno,
- "cannot create read lock in repository `%s'",
- lock->repository);
+ "cannot create read lock in repository %s",
+ quote (lock->repository));
return L_ERROR;
}
#endif /* LOCK_COMPATIBILITY */
@@ -970,7 +995,12 @@
return L_OK;
}
else
+ {
+ if (status == L_ERROR)
+ error (0, errno, "failed to obtain promotable lock in %s",
+ quote (lock->repository));
return status;
+ }
}
@@ -1018,24 +1048,23 @@
{
char *wait_repos;
- TRACE (TRACE_FLOW, "lock_list_promotably ()");
+ /* We only know how to do one list at a time */
+ assert (!locklist);
+
+ TRACE (TRACE_FLOW, "lock_list_promotably()");
if (noexec)
return 0;
- if (readonlyfs) {
- error (0, 0,
- "promotable lock failed.\n\
-WARNING: Read-only repository access mode selected via `cvs -R'.\n\
-Attempting to write to a read-only filesystem is not allowed.");
- return 1;
- }
-
- /* We only know how to do one list at a time */
- if (locklist != NULL)
+ if (readonlyfs)
{
+ char *cmd = Xasprintf ("%s -R", program_name);
error (0, 0,
- "lock_list_promotably called while promotable locks set -
Help!");
+"promotable lock failed.\n"
+"WARNING: Read-only repository access mode selected via %s.\n"
+"Attempting to write to a read-only filesystem is not allowed.",
+ quote (cmd));
+ free (cmd);
return 1;
}
@@ -1046,27 +1075,26 @@
lock_error = L_OK; /* init for set_promotablelock_proc */
lock_error_repos = NULL; /* init for set_promotablelock_proc */
locklist = list; /* init for Lock_Cleanup */
- unset_lockers_name ();
+ unset_lockers_name();
(void) walklist (list, set_promotablelock_proc, NULL);
switch (lock_error)
{
case L_ERROR: /* Real Error */
- if (wait_repos != NULL)
- free (wait_repos);
- Lock_Cleanup (); /* clean up any locks we set */
+ if (wait_repos) free (wait_repos);
+ Lock_Cleanup(); /* clean up any locks we set */
error (0, 0, "lock failed - giving up");
return 1;
case L_LOCKED: /* Someone already had a lock */
- remove_locks (); /* clean up any locks we set */
+ remove_locks(); /* clean up any locks we set */
lock_wait (lock_error_repos); /* sleep a while and try again */
wait_repos = xstrdup (lock_error_repos);
continue;
case L_OK: /* we got the locks set */
- if (wait_repos != NULL)
+ if (wait_repos)
{
lock_obtained (wait_repos);
free (wait_repos);
@@ -1074,7 +1102,7 @@
return 0;
default:
- if (wait_repos != NULL)
+ if (wait_repos)
free (wait_repos);
error (0, 0, "unknown lock status %d in lock_list_promotably",
lock_error);
@@ -1144,7 +1172,8 @@
if (lock->lockdir)
{
if (CVS_RMDIR (lock->lockdir) < 0)
- error (0, errno, "failed to remove lock dir `%s'", lock->lockdir);
+ error (0, errno, "failed to remove lock dir %s",
+ quote (lock->lockdir));
free (lock->lockdir);
lock->lockdir = NULL;
}
@@ -1253,7 +1282,7 @@
FILE *fp;
if (set_lock (&global_writelock, true) != L_OK)
- error (1, 0, "failed to obtain write lock in repository `%s'",
+ error (1, errno, "failed to obtain write lock in %s",
repository);
/* check if readers exist */
@@ -1272,25 +1301,23 @@
/* write the write-lock file */
global_writelock.file1 = lock_name (global_writelock.repository,
writelock);
- if ((fp = CVS_FOPEN (global_writelock.file1, "w+")) == NULL
+ if (!(fp = CVS_FOPEN (global_writelock.file1, "w+"))
|| fclose (fp) == EOF)
{
int xerrno = errno;
if (CVS_UNLINK (global_writelock.file1) < 0
&& !existence_error (errno))
- {
error (0, errno, "failed to remove write lock %s",
- global_writelock.file1);
- }
+ quote (global_writelock.file1));
/* free the lock dir */
clear_lock (&global_writelock);
/* return the error */
error (1, xerrno,
- "cannot create write lock in repository `%s'",
- global_writelock.repository);
+ "cannot create write lock in repository %s",
+ quote (global_writelock.repository));
}
/* If we upgraded from a promotable lock, remove it. */
@@ -1330,9 +1357,9 @@
if (set_lock (lock, true) != L_OK)
{
if (!really_quiet)
- error (0, 0,
- "failed to obtain lock `%s' in repository directory `%s'.",
- lock->lockdirname, Short_Repository (lock->repository));
+ error (0, errno,
+ "failed to obtain lock %s in repository directory %s.",
+ quote (lock->lockdirname), quote (lock->repository));
return 0;
}
Index: sanity.sh
===================================================================
RCS file: /cvsroot/cvs/ccvs/src/sanity.sh,v
retrieving revision 1.1204
retrieving revision 1.1205
diff -u -b -r1.1204 -r1.1205
--- sanity.sh 17 Sep 2008 19:53:30 -0000 1.1204
+++ sanity.sh 19 Sep 2008 00:48:07 -0000 1.1205
@@ -32044,10 +32044,9 @@
# That this is an error is good - we are asking CVS to do
# something which doesn't make sense.
dotest_fail multiroot3-10 \
-"${testcvs} -q -d ${CVSROOT1} diff dir1/file1 dir2/file2" \
-"${SPROG} diff: failed to create lock directory for .${TESTDIR}/root1/dir2'
(${TESTDIR}/root1/dir2/#cvs.lock): No such file or directory
-${SPROG} diff: failed to obtain dir lock in repository .${TESTDIR}/root1/dir2'
-${SPROG} \[diff aborted\]: read lock failed - giving up"
+"$testcvs -q -d $CVSROOT1 diff dir1/file1 dir2/file2" \
+"$SPROG diff: failed to obtain read lock in \`$TESTDIR/root1/dir2': No such
file or directory
+$SPROG \[diff aborted\]: read lock failed - giving up"
# This one is supposed to work.
dotest multiroot3-11 "${testcvs} -q diff dir1/file1 dir2/file2" ""
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Cvs-cvs] ccvs/src ChangeLog lock.c sanity.sh,
Derek Robert Price <=