Played around with the file locking and discovered to my chagrin that

whenever a process writes a cell to disk, it immediately releases the
file lock it had on that cell, which is clearly not the intent of file
locking.  Fixed this issue.  On a related topic, revised the "cellname
writeable" command so that it can make a cell editable even if the cell
has an advisory lock and cannot be made writeable.  Perhaps there should
be a clearer distinction here between "writeable" and "editable".  Also:
Reconsidered the previous commit, which removed the "--disable-locking"
from the configuration options.  Because some operating systems may not
implement fnctl()-based file locking (Cygwin, for one, apparently doesn't),
it is still useful to be able to completely remove the function, in case
the operating system will fail to recognize the fnctl() values in the
code.  Now, file locking behavior can be permanently removed through the
configuration option, or temporarily disabled from the command line.
This commit is contained in:
Tim Edwards 2022-01-01 16:53:46 -05:00
parent e4d1c29112
commit 1bb4cb92ea
9 changed files with 89 additions and 30 deletions

View File

@ -1108,23 +1108,32 @@ CmdCellname(w, cmd)
else if (subopt >= 4) else if (subopt >= 4)
{ {
/* Check if file is already read-write */ /* Check if file is already read-write */
#ifdef FILE_LOCKS
if (!(cellDef->cd_flags & CDNOEDIT) && (cellDef->cd_fd != -1))
#else
if (!(cellDef->cd_flags & CDNOEDIT)) if (!(cellDef->cd_flags & CDNOEDIT))
#endif
break; break;
/* Make file read-write */ /* Make file read-write */
#ifdef FILE_LOCKS #ifdef FILE_LOCKS
if (cellDef->cd_fd == -1) if (cellDef->cd_fd == -1)
dbReadOpen(cellDef, NULL, TRUE, NULL); dbReadOpen(cellDef, NULL, TRUE, NULL);
if (cellDef->cd_fd == -1) if (cellDef->cd_fd == -1)
TxError("Overriding advisory lock held on cell %s\n", {
TxError("An advisory lock is held on cell %s. Cell can now"
" be made editable but is not writeable.\n",
cellDef->cd_name); cellDef->cd_name);
}
#endif #endif
if (cellDef->cd_flags & CDNOEDIT)
{
cellDef->cd_flags &= ~CDNOEDIT; cellDef->cd_flags &= ~CDNOEDIT;
WindAreaChanged(w, &w->w_screenArea); WindAreaChanged(w, &w->w_screenArea);
CmdSetWindCaption(EditCellUse, EditRootDef); CmdSetWindCaption(EditCellUse, EditRootDef);
} }
}
else /* "cellname writeable false" */ else /* "cellname writeable false" */
{ {
/* Check if file is already read-only */ /* Check if file is already read-only */

View File

@ -54,8 +54,10 @@ void CmdPaintEraseButton();
extern Label *DefaultLabel; extern Label *DefaultLabel;
#ifdef FILE_LOCKS
/* See the "locking" command */ /* See the "locking" command */
extern bool FileLocking; extern bool FileLocking;
#endif
/* /*
* ---------------------------------------------------------------------------- * ----------------------------------------------------------------------------
@ -541,6 +543,7 @@ keepGoing(use, clientdata)
return 0; /* keep the search going */ return 0; /* keep the search going */
} }
#ifdef FILE_LOCKS
/* /*
* ---------------------------------------------------------------------------- * ----------------------------------------------------------------------------
* *
@ -604,6 +607,7 @@ CmdLocking(w, cmd)
FileLocking = (option <= 4) ? FALSE : TRUE; FileLocking = (option <= 4) ? FALSE : TRUE;
} }
} }
#endif /* FILE_LOCKS */
/* /*
* ---------------------------------------------------------------------------- * ----------------------------------------------------------------------------

View File

@ -69,6 +69,10 @@ static char rcsid[] __attribute__ ((unused)) = "$Header: /usr/cvsroot/magic-8.0/
extern char *Path; extern char *Path;
#ifdef FILE_LOCKS
extern bool FileLocking;
#endif
/* Suffix for all Magic files */ /* Suffix for all Magic files */
char *DBSuffix = ".mag"; char *DBSuffix = ".mag";
@ -3225,16 +3229,21 @@ DBCellWrite(cellDef, fileName)
if (cellDef->cd_flags & CDNOEDIT) if (cellDef->cd_flags & CDNOEDIT)
{ {
#ifdef FILE_LOCKS
TxPrintf("File %s is locked by another user or "
"is read_only and cannot be written\n", realname);
#else
TxPrintf("File %s is read_only and cannot be written\n", realname); TxPrintf("File %s is read_only and cannot be written\n", realname);
#endif
freeMagic(realname); freeMagic(realname);
return(FALSE); return(FALSE);
} }
#ifdef FILE_LOCKS
if (cellDef->cd_fd == -1)
{
TxPrintf("File %s is locked by another user and "
"cannot be written\n", realname);
freeMagic(realname);
return(FALSE);
}
#endif
/* Check if the .mag file exists. If not, we don't need to deal */ /* Check if the .mag file exists. If not, we don't need to deal */
/* with temporary file names. */ /* with temporary file names. */
exists = (access(expandname, F_OK) == 0) ? TRUE : FALSE; exists = (access(expandname, F_OK) == 0) ? TRUE : FALSE;
@ -3321,17 +3330,6 @@ DBCellWrite(cellDef, fileName)
tmpname, expandname, expandname, cellDef->cd_name, tmpname); tmpname, expandname, expandname, cellDef->cd_name, tmpname);
goto cleanup; goto cleanup;
} }
#ifdef FILE_LOCKS
else
{
bool dereference = (cellDef->cd_flags & CDDEREFERENCE) ? TRUE : FALSE;
/* Re-aquire the lock on the new file by opening it. */
if (DBCellRead(cellDef, NULL, TRUE, dereference, NULL) == FALSE)
return FALSE;
}
#endif
} }
else if (exists) else if (exists)
{ {
@ -3408,7 +3406,14 @@ DBCellWrite(cellDef, fileName)
result = TRUE; result = TRUE;
{ {
struct stat thestat; struct stat thestat;
bool is_locked;
#ifdef FILE_LOCKS
if (FileLocking)
realf = flock_open(expandname, "r", &is_locked);
else
#endif
realf = fopen(expandname, "r"); realf = fopen(expandname, "r");
if (realf == NULL) if (realf == NULL)
{ {
cellDef->cd_flags |= CDMODIFIED; cellDef->cd_flags |= CDMODIFIED;
@ -3416,12 +3421,19 @@ DBCellWrite(cellDef, fileName)
} }
else else
{ {
fstat(fileno(realf),&thestat); int fd = fileno(realf);
fstat(fd, &thestat);
if (thestat.st_size != DBFileOffset) if (thestat.st_size != DBFileOffset)
{ {
cellDef->cd_flags |= CDMODIFIED; cellDef->cd_flags |= CDMODIFIED;
TxError("Warning: I/O error in writing file \"%s\"\n", expandname); TxError("Warning: I/O error in writing file \"%s\"\n", expandname);
} }
#ifdef FILE_LOCKS
if (FileLocking && (is_locked == FALSE))
cellDef->cd_fd = fd;
else
#endif
fclose(realf); fclose(realf);
} }
realf = NULL; realf = NULL;

View File

@ -46,7 +46,10 @@ extern void CmdDelete(), CmdDown(), CmdDrc(), CmdDrop(), CmdDump();
extern void CmdEdit(), CmdElement(), CmdErase(), CmdExpand(), CmdExtract(); extern void CmdEdit(), CmdElement(), CmdErase(), CmdExpand(), CmdExtract();
extern void CmdFeedback(), CmdFill(), CmdFindBox(), CmdFindLabel(), CmdFlush(); extern void CmdFeedback(), CmdFill(), CmdFindBox(), CmdFindLabel(), CmdFlush();
extern void CmdGetcell(), CmdGrid(), CmdIdentify(); extern void CmdGetcell(), CmdGrid(), CmdIdentify();
extern void CmdLabel(), CmdLoad(), CmdLocking(); extern void CmdLabel(), CmdLoad();
#ifdef FILE_LOCKS
extern void CmdLocking();
#endif
extern void CmdMove(), CmdNetlist(), CmdOrient(), CmdPaint(), CmdPath(); extern void CmdMove(), CmdNetlist(), CmdOrient(), CmdPaint(), CmdPath();
extern void CmdPlow(), CmdPolygon(), CmdPort(), CmdProperty(); extern void CmdPlow(), CmdPolygon(), CmdPort(), CmdProperty();
extern void CmdRandom(), CmdSave(), CmdScaleGrid(), CmdSee(); extern void CmdRandom(), CmdSave(), CmdScaleGrid(), CmdSee();
@ -369,9 +372,11 @@ DBWInitCommands()
WindAddCommand(DBWclientID, WindAddCommand(DBWclientID,
"load [cellname] load a cell into a window", "load [cellname] load a cell into a window",
CmdLoad, FALSE); CmdLoad, FALSE);
#ifdef FILE_LOCKS
WindAddCommand(DBWclientID, WindAddCommand(DBWclientID,
"locking [enable|disable] enable or disable file locking", "locking [enable|disable] enable or disable file locking",
CmdLocking, FALSE); CmdLocking, FALSE);
#endif
WindAddCommand(DBWclientID, WindAddCommand(DBWclientID,
"move [dir [amount]] OR\n" "move [dir [amount]] OR\n"
"move to x y move box and selection, either by amount\n\ "move to x y move box and selection, either by amount\n\

11
scripts/configure vendored
View File

@ -6844,8 +6844,15 @@ else
fi fi
if test "x$enable_locking" = "xno" ; then if test "x$enable_locking" = "xyes" ; then
echo "File locking no longer set during configuration; ignoring option." case $target in
*cygwin*)
;;
*)
$as_echo "#define FILE_LOCKS 1" >>confdefs.h
;;
esac
fi fi
# Check whether --enable-calma was given. # Check whether --enable-calma was given.

View File

@ -935,8 +935,14 @@ AC_ARG_ENABLE(locking,
[], [],
[enable_locking=yes]) [enable_locking=yes])
if test "x$enable_locking" = "xno" ; then if test "x$enable_locking" = "xyes" ; then
echo "File locking no longer set during configuration; ignoring option." case $target in
*cygwin*)
;;
*)
AC_DEFINE(FILE_LOCKS)
;;
esac
fi fi
AC_ARG_ENABLE(calma, AC_ARG_ENABLE(calma,

View File

@ -11,6 +11,8 @@
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
#ifdef FILE_LOCKS
#include <unistd.h> #include <unistd.h>
#include <fcntl.h> #include <fcntl.h>
@ -164,3 +166,5 @@ FILE *flock_open(filename, mode, is_locked)
done: done:
return f; return f;
} }
#endif /* FILE_LOCKS */

View File

@ -46,7 +46,9 @@ static char rcsid[] __attribute__ ((unused)) = "$Header: /usr/cvsroot/magic-8.0/
static HashTable expansionTable; static HashTable expansionTable;
static bool noTable = TRUE; static bool noTable = TRUE;
#ifdef FILE_LOCKS
bool FileLocking = TRUE; bool FileLocking = TRUE;
#endif
/* Limit on how long a single file name may be: */ /* Limit on how long a single file name may be: */
@ -456,9 +458,11 @@ PaLockOpen(file, mode, ext, path, library, pRealName, is_locked)
p2 = file; p2 = file;
if (PaExpand(&p2, &p1, MAXSIZE) < 0) return NULL; if (PaExpand(&p2, &p1, MAXSIZE) < 0) return NULL;
#ifdef FILE_LOCKS
if (FileLocking) if (FileLocking)
return flock_open(realName, mode, is_locked); return flock_open(realName, mode, is_locked);
else else
#endif
return fopen(realName, mode); return fopen(realName, mode);
} }
@ -475,9 +479,11 @@ PaLockOpen(file, mode, ext, path, library, pRealName, is_locked)
(void) strncpy(realName, file, MAXSIZE-1); (void) strncpy(realName, file, MAXSIZE-1);
realName[MAXSIZE-1] = '\0'; realName[MAXSIZE-1] = '\0';
#ifdef FILE_LOCKS
if (FileLocking) if (FileLocking)
return flock_open(realName, mode, is_locked); return flock_open(realName, mode, is_locked);
else else
#endif
return fopen(realName, mode); return fopen(realName, mode);
} }
@ -487,9 +493,11 @@ PaLockOpen(file, mode, ext, path, library, pRealName, is_locked)
{ {
if (*realName == 0) continue; if (*realName == 0) continue;
#ifdef FILE_LOCKS
if (FileLocking) if (FileLocking)
f = flock_open(realName, mode, is_locked); f = flock_open(realName, mode, is_locked);
else else
#endif
f = fopen(realName, mode); f = fopen(realName, mode);
if (f != NULL) return f; if (f != NULL) return f;
@ -506,9 +514,11 @@ PaLockOpen(file, mode, ext, path, library, pRealName, is_locked)
if (library == NULL) return NULL; if (library == NULL) return NULL;
while (nextName(&library, file, realName, MAXSIZE) != NULL) while (nextName(&library, file, realName, MAXSIZE) != NULL)
{ {
#ifdef FILE_LOCKS
if (FileLocking) if (FileLocking)
f = flock_open(realName, mode, is_locked); f = flock_open(realName, mode, is_locked);
else else
#endif
f = fopen(realName, mode); f = fopen(realName, mode);
if (f != NULL) return f; if (f != NULL) return f;

View File

@ -54,7 +54,9 @@ extern bool StrIsNumeric(char *);
extern int SetNoisyBool(bool *, char *, FILE *); extern int SetNoisyBool(bool *, char *, FILE *);
#ifdef FILE_LOCKS
extern FILE *flock_open(); extern FILE *flock_open();
#endif
/* The following macro takes an integer and returns another integer that /* The following macro takes an integer and returns another integer that
* is the same as the first except that all the '1' bits are turned off, * is the same as the first except that all the '1' bits are turned off,