lockdev: FTBFS on hurd-i386
Source: lockdev
Version: 1.0.3-1.4
Severity: important
Tags: patch
User: debian-hurd@lists.debian.org
Usertags: hurd
Hi,
This patch solves the build problems for GNU/Hurd due to MAXPATHLEN
issues. The solution is to make dynamic string allocations instead of
using fixed length buffers in the file src/lockdev.c Another change is
the major and minor names in <sys/sysmacros.h> are different on GNU/Hurd
compared to other systems.
Finally for the tests in LockDev/test.pl using /dev/ttyS3 does not exist
on GNU/Hurd, and it is replaced by the for all systems common
device /dev/tty6.
The whole package has been tested with valgrind on GNU/Linux, with heap
errors and memory leaks but they are not due to the patch, they are
already present in the non-patched version, emanating from perl.
TODO:
Check that all free() statements are correctly placed.
Thanks!
(Not submitted yet, comments are welcome.)
diff -ur lockdev-1.0.3/LockDev/test.pl lockdev-1.0.3.modified/LockDev/test.pl
--- lockdev-1.0.3/LockDev/test.pl 1999-12-01 13:21:56.000000000 +0100
+++ lockdev-1.0.3.modified/LockDev/test.pl 2012-01-25 19:06:59.000000000 +0100
@@ -19,11 +19,11 @@
# of the test code):
# 2 no lock
-print &LockDev::dev_testlock( "/dev/ttyS3") == 0 ? "ok 2\n" : "not ok 2\n";
-print &LockDev::dev_lock( "/dev/ttyS3") == 0 ? "ok 3\n" : "not ok 3\n";
-print &LockDev::dev_testlock( "/dev/ttyS3") == $$ ? "ok 4\n" : "not ok 4\n";
-print &LockDev::dev_relock( "/dev/ttyS3", 0) == 0 ? "ok 5\n" : "not ok 5\n";
-print &LockDev::dev_testlock( "/dev/ttyS3") == $$ ? "ok 6\n" : "not ok 6\n";
-print &LockDev::dev_unlock( "/dev/ttyS3", $$) == 0 ? "ok 7\n" : "not ok 7\n";
-print &LockDev::dev_testlock( "/dev/ttyS3") == 0 ? "ok 8\n" : "not ok 8\n";
+print &LockDev::dev_testlock( "/dev/tty6") == 0 ? "ok 2\n" : "not ok 2\n";
+print &LockDev::dev_lock( "/dev/tty6") == 0 ? "ok 3\n" : "not ok 3\n";
+print &LockDev::dev_testlock( "/dev/tty6") == $$ ? "ok 4\n" : "not ok 4\n";
+print &LockDev::dev_relock( "/dev/tty6", 0) == 0 ? "ok 5\n" : "not ok 5\n";
+print &LockDev::dev_testlock( "/dev/tty6") == $$ ? "ok 6\n" : "not ok 6\n";
+print &LockDev::dev_unlock( "/dev/tty6", $$) == 0 ? "ok 7\n" : "not ok 7\n";
+print &LockDev::dev_testlock( "/dev/tty6") == 0 ? "ok 8\n" : "not ok 8\n";
diff -ur lockdev-1.0.3/src/lockdev.c lockdev-1.0.3.modified/src/lockdev.c
--- lockdev-1.0.3/src/lockdev.c 2006-04-20 21:11:08.000000000 +0200
+++ lockdev-1.0.3.modified/src/lockdev.c 2012-01-26 10:12:22.000000000 +0100
@@ -123,7 +123,11 @@
#include <sys/param.h>
#include <sys/stat.h>
#include <sys/file.h>
-#if defined (__GNU_LIBRARY__)
+#if defined (__GNU__)
+# include <sys/sysmacros.h>
+# define MAJOR(dev) major (dev)
+# define MINOR(dev) minor (dev)
+#elif defined (__GNU_LIBRARY__)
# include <sys/sysmacros.h>
# define MAJOR(dev) gnu_dev_major (dev)
# define MINOR(dev) gnu_dev_minor (dev)
@@ -138,9 +142,9 @@
* all internal functions names start with _dl_
*/
-static inline int _dl_filename_0 ( char * name, const pid_t pid);
-static inline int _dl_filename_1 ( char * name, const struct stat * st);
-static inline int _dl_filename_2 ( char * name, const char * dev);
+static inline int _dl_filename_0 ( char ** name, const pid_t pid);
+static inline int _dl_filename_1 ( char ** name, const struct stat * st);
+static inline int _dl_filename_2 ( char ** name, const char * dev);
static void _dl_sig_handler ( int sig);
static int _dl_get_semaphore ( int flag);
static int _dl_unlock_semaphore ( int value);
@@ -193,47 +197,56 @@
/* for internal use */
static inline int
-_dl_filename_0 (char *name,
+_dl_filename_0 (char **name,
const pid_t pid)
{
- int l;
+ int l, len;
_debug( 3, "_dl_filename_0 (pid=%d)\n", (int)pid);
/* file of type /var/lock/LCK...<pid> */
- l = sprintf( name, "%s/LCK...%d", LOCK_PATH, (int)pid);
- _debug( 2, "_dl_filename_0 () -> len=%d, name=%s<\n", l, name);
+ len = strlen( LOCK_PATH) + 7 + 3*sizeof( int) + 1;
+ if ( (*name = malloc( len)) == NULL )
+ return -1;
+ l = sprintf( *name, "%s/LCK...%d", LOCK_PATH, (int)pid);
+ _debug( 2, "_dl_filename_0 () -> len=%d, name=%s<\n", l, *name);
return l;
}
/* for internal use */
static inline int
-_dl_filename_1 (char *name,
+_dl_filename_1 (char **name,
const struct stat *st)
{
- int l;
+ int l, len;
int add = 0;
_debug( 3, "_dl_filename_1 (mode=%d, stat=%d)\n",
(int)st->st_mode, (int)st->st_rdev);
/* lockfile of type /var/lock/LCK.C.004.064 */
- l = sprintf( name, "%s/LCK.%c.%03d.%03d", LOCK_PATH,
+ len = strlen( LOCK_PATH) + 7 + 2*3*sizeof( int) + 1;
+ if ( (*name = malloc( len)) == NULL )
+ return -1;
+ l = sprintf( *name, "%s/LCK.%c.%03d.%03d", LOCK_PATH,
S_ISCHR(st->st_mode) ? 'C' : S_ISBLK(st->st_mode) ? 'B' : 'X',
(int)MAJOR( add+st->st_rdev), (int)MINOR( add+st->st_rdev));
- _debug( 2, "_dl_filename_1 () -> len=%d, name=%s<\n", l, name);
+ _debug( 2, "_dl_filename_1 () -> len=%d, name=%s<\n", l, *name);
return l;
}
/* for internal use */
static inline int
-_dl_filename_2 (char *name,
+_dl_filename_2 (char **name,
const char *dev)
{
- int l, i;
+ int l, i, len;
_debug( 3, "_dl_filename_2 (dev=%s)\n", dev);
/* lockfile of type /var/lock/LCK..ttyS2 */
- l = sprintf( name, "%s/LCK..%s", LOCK_PATH, dev);
- _debug( 2, "_dl_filename_2 () -> len=%d, name=%s<\n", l, name);
- for (i=strlen(LOCK_PATH)+1; name[i] != '\0'; ++i) {
- if (name[i] == '/')
- name[i] = ':';
+ len = strlen( LOCK_PATH) + 6 + strlen( dev) + 1;
+ if ( (*name = malloc( len)) == NULL )
+ return -1;
+ l = snprintf( *name, len, "%s/LCK..%s", LOCK_PATH, dev);
+ _debug( 2, "_dl_filename_2 () -> len=%d, name=%s<\n", l, *name);
+ for (i=strlen(LOCK_PATH)+1; (*name)[i] != '\0'; ++i) {
+ if ((*name)[i] == '/')
+ (*name)[i] = ':';
}
return l;
}
@@ -265,8 +278,8 @@
* its positive conten doesn't tells that we lock the file.
*/
static int semaphore= -1;
-static char sem_name[MAXPATHLEN+1];
-static int oldmask= -1;
+static char *sem_name = NULL;
+static int oldmask= -1, len;
/* for internal use */
static int
@@ -274,7 +287,10 @@
{
int flag2 = flag;
- sprintf( sem_name, "%s/%s", LOCK_PATH, SEMAPHORE);
+ len = strlen( LOCK_PATH) + 1 + strlen( SEMAPHORE) + 1;
+ if ( (sem_name = malloc( len)) == NULL )
+ return 0;
+ snprintf( sem_name, len, "%s/%s", LOCK_PATH, SEMAPHORE);
_debug( 1, "_dl_get_semaphore(name=%s)=%d\n", sem_name, semaphore);
while ( semaphore == -1 ) {
semaphore = creat( sem_name, (S_IRWXU | S_IRWXG | S_IRWXO));
@@ -326,6 +342,7 @@
if ( semaphore != -1 ) {
_debug( 1, "_dl_unlock_semaphore(return=%d)=%d\n", value, semaphore);
unlink( sem_name);
+ free( sem_name);
close( semaphore);
semaphore = -1;
}
@@ -416,14 +433,17 @@
* ... why should we care more about this?
*/
{
- char tpname[MAXPATHLEN+1];
+ char *tpname = NULL;
pid_t pid2 = 0;
/* make a unique filename, but different from anyone else.
- * maybe also this sprintf should be added to the
+ * maybe also this snprintf should be added to the
* conditional part, as the others
*/
- sprintf( tpname, "%s/.%d", LOCK_PATH, (int)getpid());
+ len = strlen( LOCK_PATH) + 2 + sizeof( int) + 1;
+ if ( (tpname = malloc( len)) == NULL )
+ return 0;
+ snprintf( tpname, len, "%s/.%d", LOCK_PATH, (int)getpid());
unlink( tpname); /* in case there was */
rename( lockname, tpname);
if ( ! (fd=fopen( tpname, "r")) ) {
@@ -445,10 +465,12 @@
_debug( 2, "_dl_check_lock() locked by %d\n", (int)pid2);
fclose( fd);
unlink( tpname);
+ free( tpname);
return( pid2);
}
fclose( fd);
unlink( tpname);
+ free( tpname);
}
_debug( 2, "_dl_check_lock() no lock\n");
return 0;
@@ -481,7 +503,7 @@
}
if ( strcmp( p, "tty") == 0 )
p = ttyname( 0); /* this terminal, if it exists */
- if ( ((l=strlen( p)) == 0 ) || ( l > (MAXPATHLEN - strlen(LOCK_PATH)) ))
+ if ( (l=strlen( p)) == 0 )
return 0;
if ( ! (m = malloc( 1 + l)) )
return 0;
@@ -495,10 +517,11 @@
dev_testlock(const char *devname)
{
const char * p;
- char device[MAXPATHLEN+1];
- char lock[MAXPATHLEN+1];
+ char * device = NULL;
+ char * lock = NULL;
struct stat statbuf;
pid_t pid;
+ int len;
#if DEBUG
if ( env_var_debug == -1 ) {
@@ -512,14 +535,17 @@
_debug( 3, "dev_testlock(%s)\n", devname);
if ( ! (p=_dl_check_devname( devname)) )
close_n_return( -1);
- strcpy( device, DEV_PATH);
- strcat( device, p); /* now device has a copy of the pathname */
+ len = strlen( DEV_PATH) + strlen( p) + 1;
+ if ( (device = malloc( len)) == NULL )
+ close_n_return(0);
+ snprintf( device, len, "%s%s", DEV_PATH, p); /* now device has a copy of the pathname */
_debug( 2, "dev_testlock() device = %s\n", device);
/* check the device name for existence and retrieve the major
* and minor numbers
*/
if ( stat( device, &statbuf) == -1 ) {
+ free( device);
close_n_return( -1);
}
@@ -528,17 +554,19 @@
* return the pid of the owner of the lock.
*/
/* lockfile of type /var/lock/LCK..ttyS2 */
- _dl_filename_2( lock, p);
- if ( (pid=_dl_check_lock( lock)) )
+ _dl_filename_2( &lock, p);
+ if ( (pid=_dl_check_lock( lock)) ) {
+ free( device);
+ free( lock);
close_n_return( pid);
-
+ }
/* and also check if a pid file was left around
* do this before the static var is wiped
* BUT! we don't fail in case the process exists, as this is not
* a lock file, just a pid holder!
*/
if ( pid_read ) {
- _dl_filename_0( lock, pid_read);
+ _dl_filename_0( &lock, pid_read);
_dl_check_lock( lock);
}
@@ -548,31 +576,36 @@
* the contrary; anyway we do both tests.
*/
/* lockfile of type /var/lock/LCK.004.064 */
- _dl_filename_1( lock, &statbuf);
- if ( (pid=_dl_check_lock( lock)) )
+ _dl_filename_1( &lock, &statbuf);
+ if ( (pid=_dl_check_lock( lock)) ) {
+ free( device);
+ free( lock);
close_n_return( pid);
+ }
if ( pid_read ) {
- _dl_filename_0( lock, pid_read);
+ _dl_filename_0( &lock, pid_read);
_dl_check_lock( lock);
}
-
+ free( device);
+ free( lock);
close_n_return( 0);
}
-
+/* DONE TO HERE: srs */
/* exported by the interface file lockdev.h */
pid_t
dev_lock (const char *devname)
{
const char * p;
- char device[MAXPATHLEN+1];
- char lock[MAXPATHLEN+1];
- char lock0[MAXPATHLEN+1];
- char lock1[MAXPATHLEN+1];
- char lock2[MAXPATHLEN+1];
+ char * device = NULL;
+ char * lock;
+ char * lock0;
+ char * lock1;
+ char * lock2;
struct stat statbuf;
pid_t pid, pid2, our_pid;
FILE *fd = 0;
+ int len;
#if DEBUG
if ( env_var_debug == -1 ) {
@@ -588,16 +621,22 @@
oldmask = umask( 0); /* give full permissions to files created */
if ( ! (p=_dl_check_devname( devname)) )
close_n_return( -1);
- strcpy( device, DEV_PATH);
- strcat( device, p); /* now device has a copy of the pathname */
+
+ /* FIXME strlen(p)?? */
+ len = strlen( DEV_PATH) + strlen( p) + 1;
+ if ( (device = malloc( len)) == NULL )
+ close_n_return(0);
+ snprintf( device, len, "%s%s", DEV_PATH, p); /* now device has a copy of the pathname */
_debug( 2, "dev_lock() device = %s\n", device);
/* check the device name for existence and retrieve the major
* and minor numbers
*/
if ( stat( device, &statbuf) == -1 ) {
+ free( device);
close_n_return( -1);
}
+ free( device);
/* now get our own pid */
our_pid = getpid();
@@ -611,9 +650,11 @@
* this operations succeed we own the lock.
*/
/* file of type /var/lock/LCK..<pid> */
- _dl_filename_0( lock0, our_pid);
- if ( ! (fd=fopen( lock0, "w")) )
+ _dl_filename_0( &lock0, our_pid);
+ if ( ! (fd=fopen( lock0, "w")) ) {
+ free( lock0);
close_n_return( -1); /* no file, no lock */
+ }
fprintf( fd, "%10d\n", (int)our_pid);
fclose( fd);
@@ -622,14 +663,16 @@
* return the pid of the owner of the lock.
*/
/* lockfile of type /var/lock/LCK..ttyS2 */
- _dl_filename_2( lock2, p);
+ _dl_filename_2( &lock2, p);
pid = _dl_check_lock( lock2);
if ( pid && pid != our_pid ) {
unlink( lock0);
+ free( lock0);
+
close_n_return( pid); /* error or locked by someone else */
}
if ( pid_read ) { /* modifyed by _dl_check_lock() */
- _dl_filename_0( lock, pid_read);
+ _dl_filename_0( &lock, pid_read);
_dl_check_lock( lock); /* remove stale pid file */
}
/* not locked or already owned by us */
@@ -638,20 +681,23 @@
* lock happens
*/
/* lockfile of type /var/lock/LCK.004.064 */
- _dl_filename_1( lock1, &statbuf);
+ _dl_filename_1( &lock1, &statbuf);
while ( ! (pid=_dl_check_lock( lock1)) ) {
if (( link( lock0, lock1) == -1 ) && ( errno != EEXIST )) {
unlink( lock0);
+ free( lock0);
+ free( lock1);
close_n_return( -1);
}
}
if ( pid != our_pid ) {
/* error or another one owns it now */
unlink( lock0);
+ free( lock0);
close_n_return( pid);
}
if ( pid_read ) { /* modifyed by _dl_check_lock() */
- _dl_filename_0( lock, pid_read);
+ _dl_filename_0( &lock, pid_read);
_dl_check_lock( lock); /* remove stale pid file */
}
/* from this point lock1 is OUR! */
@@ -661,6 +707,8 @@
if (( link( lock0, lock2) == -1 ) && ( errno != EEXIST )) {
unlink( lock0);
unlink( lock1);
+ free( lock0);
+ free( lock1);
close_n_return( -1);
}
}
@@ -672,11 +720,13 @@
*/
unlink( lock0);
unlink( lock1);
+ free( lock0);
+ free( lock1);
close_n_return( pid);
}
/* quite unlike, but ... */
if ( pid_read ) { /* modifyed by _dl_check_lock() */
- _dl_filename_0( lock, pid_read);
+ _dl_filename_0( &lock, pid_read);
_dl_check_lock( lock); /* remove stale pid file */
}
@@ -694,15 +744,19 @@
pid2 = _dl_check_lock( lock2);
if (( pid == pid2 ) && ( pid == our_pid )) {
_debug( 2, "dev_lock() got lock\n");
+ free( lock1);
+ free( lock2);
close_n_return( 0); /* locked by us */
}
/* oh, no! someone else stepped in! */
if ( pid == our_pid ) {
unlink( lock1);
+ free( lock1);
pid = 0;
}
if ( pid2 == our_pid ) {
unlink( lock2);
+ free( lock2);
pid2 = 0;
}
if ( pid && pid2 ) {
@@ -710,6 +764,9 @@
_debug( 1, "dev_lock() process %d owns file %s\n", (int)pid, lock1);
_debug( 1, "dev_lock() process %d owns file %s\n", (int)pid2, lock2);
_debug( 1, "dev_lock() process %d (we) have no lock!\n", (int)our_pid);
+ free( lock0);
+ free( lock1);
+ free( lock2);
close_n_return( -1);
}
close_n_return( (pid + pid2));
@@ -721,13 +778,14 @@
dev_relock (const char *devname,
const pid_t old_pid)
{
- const char * p;
- char device[MAXPATHLEN+1];
- char lock1[MAXPATHLEN+1];
- char lock2[MAXPATHLEN+1];
+ const char *p;
+ char *device;
+ char * lock1;
+ char * lock2;
struct stat statbuf;
pid_t pid, our_pid;
FILE *fd = 0;
+ int len;
#if DEBUG
if ( env_var_debug == -1 ) {
@@ -743,14 +801,17 @@
oldmask = umask( 0); /* give full permissions to files created */
if ( ! (p=_dl_check_devname( devname)) )
close_n_return( -1);
- strcpy( device, DEV_PATH);
- strcat( device, p); /* now device has a copy of the pathname */
+ len = strlen( DEV_PATH) + strlen( p) + 1;
+ if ( (device = malloc( len)) == NULL )
+ close_n_return(0);
+ snprintf( device, len, "%s%s", DEV_PATH, p); /* now device has a copy of the pathname */
_debug( 2, "dev_relock() device = %s\n", device);
/* check the device name for existence and retrieve the major
* and minor numbers
*/
if ( stat( device, &statbuf) == -1 ) {
+ free( device);
close_n_return( -1);
}
@@ -763,37 +824,52 @@
* return the pid of the owner of the lock.
*/
/* lockfile of type /var/lock/LCK..ttyS2 */
- _dl_filename_2( lock2, p);
+ _dl_filename_2( &lock2, p);
pid = _dl_check_lock( lock2);
- if ( pid && old_pid && pid != old_pid )
+ if ( pid && old_pid && pid != old_pid ) {
+ free( device);
+ free( lock2);
close_n_return( pid); /* error or locked by someone else */
-
+ }
/* lockfile of type /var/lock/LCK.004.064 */
- _dl_filename_1( lock1, &statbuf);
+ _dl_filename_1( &lock1, &statbuf);
pid = _dl_check_lock( lock1);
- if ( pid && old_pid && pid != old_pid )
+ if ( pid && old_pid && pid != old_pid ) {
+ free( device);
+ free( lock1);
close_n_return( pid); /* error or locked by someone else */
-
+ }
if ( ! pid ) { /* not locked ??? */
/* go and lock it */
+ free( device);
+ free( lock1);
return( dev_lock( devname));
}
/* we don't rewrite the pids in the lock files untill we're sure
* we own all the lockfiles
*/
- if ( ! (fd=fopen( lock1, "w")) )
+ if ( ! (fd=fopen( lock1, "w")) ) {
+ free( device);
+ free( lock1);
close_n_return( -1); /* something strange */
+ }
fprintf( fd, "%10d\n", (int)our_pid);
+ free( lock1);
fclose( fd);
/* under normal conditions, this second file is a hardlink of
* the first, so we have yet modifyed it also, and this write
* seems redundant ... but ... doesn't hurt.
*/
- if ( ! (fd=fopen( lock2, "w")) )
+ if ( ! (fd=fopen( lock2, "w")) ) {
/* potentially a problem */
+ free( device);
+ free( lock2);
close_n_return( -1); /* something strange */
+ }
fprintf( fd, "%10d\n", (int)our_pid);
+ free( device);
+ free( lock2);
fclose( fd);
_debug( 2, "dev_relock() lock changed\n");
@@ -807,12 +883,13 @@
const pid_t pid)
{
const char * p;
- char device[MAXPATHLEN+1];
- char lock0[MAXPATHLEN+1];
- char lock1[MAXPATHLEN+1];
- char lock2[MAXPATHLEN+1];
+ char * device;
+ char * lock0;
+ char * lock1;
+ char * lock2;
struct stat statbuf;
pid_t wpid;
+ int len;
#if DEBUG
if ( env_var_debug == -1 ) {
@@ -828,14 +905,18 @@
oldmask = umask( 0); /* give full permissions to files created */
if ( ! (p=_dl_check_devname( devname)) )
close_n_return( -1);
- strcpy( device, DEV_PATH);
- strcat( device, p); /* now device has a copy of the pathname */
+ /* FIXME strlen(p)?? */
+ len = strlen( DEV_PATH) + strlen( p) + 1;
+ if ( (device = malloc( len)) == NULL )
+ close_n_return(0);
+ snprintf( device, len, "%s%s", DEV_PATH, p); /* now device has a copy of the pathname */
_debug( 2, "dev_unlock() device = %s\n", device);
/* check the device name for existence and retrieve the major
* and minor numbers
*/
if ( stat( device, &statbuf) == -1 ) {
+ free( device);
close_n_return( -1);
}
@@ -844,26 +925,35 @@
* return the pid of the owner of the lock.
*/
/* lockfile of type /var/lock/LCK..ttyS2 */
- _dl_filename_2( lock2, p);
+ _dl_filename_2( &lock2, p);
wpid = _dl_check_lock( lock2);
- if ( pid && wpid && pid != wpid )
+ if ( pid && wpid && pid != wpid ) {
+ free( device);
+ free( lock2);
close_n_return( wpid); /* error or locked by someone else */
-
+ }
/* lockfile of type /var/lock/LCK.004.064 */
- _dl_filename_1( lock1, &statbuf);
+ _dl_filename_1( &lock1, &statbuf);
wpid = _dl_check_lock( lock1);
- if ( pid && wpid && pid != wpid )
+ if ( pid && wpid && pid != wpid ) {
+ free( device);
+ free( lock1);
close_n_return( wpid); /* error or locked by someone else */
-
- _dl_filename_0( lock0, wpid);
- if ( wpid == _dl_check_lock( lock0))
+ }
+ _dl_filename_0( &lock0, wpid);
+ if ( wpid == _dl_check_lock( lock0)) {
unlink( lock0);
+ free( lock0);
+ }
/* anyway now we remove the files, in the reversed order than
* they have been built.
*/
+ free( device);
unlink( lock2);
unlink( lock1);
+ free( lock2);
+ free( lock1);
_debug( 2, "dev_unlock() unlocked\n");
close_n_return( 0); /* successfully unlocked */
}
Reply to: