[paranoia-dev] gcc warning cleanups, exclusive opening of devices

Peter Jones pjones at redhat.com
Mon Dec 8 09:58:34 PST 2003



Hi everybody. 

Red Hat's builds of cdparanoia currently have 3 patches which aren't in 
the main CVS tree.  I'd like to run these by you guys, and if nobody has 
anything against them, I can put them in CVS.

The first is a cleanup of interface/utils.h to remove some gcc3 warning 
messages.  It doesn't change any actual behavior at all, just some C 
style.  GCC doesn't like labels in switches that don't have any associated 
code.

--- cdparanoia-III-alpha9.8/interface/utils.h.labels    2003-05-06 10:35:43.000000000 -0400
+++ cdparanoia-III-alpha9.8/interface/utils.h   2003-05-06 10:36:05.000000000 -0400
@@ -110,8 +110,6 @@
     case CDDA_MESSAGE_LOGIT:
       d->errorbuf=catstring(d->errorbuf,s);
       break;
-    case CDDA_MESSAGE_FORGETIT:
-    default:
     }
   }
 }
@@ -125,8 +123,6 @@
     case CDDA_MESSAGE_LOGIT:
       d->messagebuf=catstring(d->messagebuf,s);
       break;
-    case CDDA_MESSAGE_FORGETIT:
-    default:
     }
   }
 }
@@ -167,8 +163,6 @@
        }
       }
       break;
-    case CDDA_MESSAGE_FORGETIT:
-    default:
     }
   }
   if(malloced)free(buffer);
@@ -203,8 +197,6 @@
        if(!malloced)*messages=catstring(*messages,"\n");
        }
       break;
-    case CDDA_MESSAGE_FORGETIT:
-    default:
     }
   }
   if(malloced)free(buffer);

<p>The second patch is removal of "strip".  We prefer to have the distro 
build system choose when to strip binaries and when not to.

--- cdparanoia-III-alpha9.8/Makefile.in.nostrip Mon Jun 17 16:01:55 2002
+++ cdparanoia-III-alpha9.8/Makefile.in Tue Mar 27 17:46:58 2001
@@ -44,7 +44,6 @@
        cd interface && $(MAKE) all
        cd paranoia && $(MAKE) all
        $(MAKE) cdparanoia CFLAGS="$(OPT)"
-       strip cdparanoia
                                                                                
 debug:
        cd interface && $(MAKE) debug

<p>The third patch makes it so that cdparanoia opens the relevant cd devices
with O_EXCL, so that other programs (which follow the convention) won't 
mess with the CD drive and screw up what we're doing.  Currently, all 
utilities that access CD devices in Fedora Core 1 and RHEL3 follow this 
convention, and e.g. grip has been fixed so that it doesn't have the FD 
open when it calls cdparanoia.

I don't know what other distros have or haven't updated their utilities so 
that this will succeed.

--- cdparanoia-III-alpha9.8/interface/scan_devices.c.O_EXCL     2001-03-26 
00:44:01.000000000 -0500
+++ cdparanoia-III-alpha9.8/interface/scan_devices.c    2003-05-20 
14:33:16.000000000 -0400
@@ -148,7 +148,7 @@
                                                                                
   cdrom_drive *d=NULL;
   struct stat st;
-  int fd=-1;
+  int fd=-1, i;
   int type;
   char *description=NULL;
   char *device;
@@ -180,7 +180,12 @@
     /* Yay, ATAPI... */
     /* Ping for CDROM-ness */
                                                                                
-    fd=open(device,O_RDONLY|O_NONBLOCK);
+    fd=open(device,O_RDONLY|O_NONBLOCK|O_EXCL);
+    for (i = 0; (i<10) && (fd == -1); i++) {
+      fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 second.\n", device, strerror(errno));
+      usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+      fd = open(device,O_RDONLY|O_NONBLOCK|O_EXCL);
+    }
     if(fd==-1){
       idperror(messagedest,messages,"\t\tUnable to open %s",device);
       free(device);
@@ -250,14 +255,6 @@
     return(NULL);
   }
                                                                                
-  if(fd==-1)fd=open(device,O_RDONLY|O_NONBLOCK);
-  if(fd==-1){
-    idperror(messagedest,messages,"\t\tUnable to open %s",device);
-    free(device);
-    if(description)free(description);
-    return(NULL);
-  }
-
   /* Minimum init */
                                                                                
   d=calloc(1,sizeof(cdrom_drive));
@@ -311,12 +308,19 @@
                        char *devfs_test,
                        char *devfs_other,
                        char *prompt,int messagedest,char **messages){
-  int dev=open(device,O_RDONLY|O_NONBLOCK);
+  int dev=-1;
   scsiid a,b;
                                                                                
   int i,j;
   char buffer[200];                                                                                 
+  dev=open(device,O_RDONLY|O_NONBLOCK|O_EXCL);
+  for (i = 0; (i<10) && (dev == -1); i++) {
+    fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 second.\n", device, strerror(errno));
+    usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+    dev = open(device,O_RDONLY|O_NONBLOCK|O_EXCL);
+  } +
   /* if we're running under /devfs, build the device name from the
      device we already have */
   if(!strncmp(device,devfs_test,strlen(devfs_test))){
@@ -327,6 +331,11 @@
       int matchf;
       sprintf(pos,"/%s",devfs_other);
       matchf=open(buffer,O_RDONLY|O_NONBLOCK);
+      for (i = 0; (i<10) && (matchf==-1); i++) { 
+        fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 seconds.\n", buffer, strerror(errno));
+        usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+        matchf = open(buffer,O_RDONLY|O_NONBLOCK);
+      }
       if(matchf!=-1){
        close(matchf);
        close(dev);
@@ -353,7 +362,7 @@
   for(i=0;i<25;i++){
     for(j=0;j<2;j++){
       int pattern=0;
-      int matchf;
+      int matchf, k;
                                                                                
       while(prefixes[pattern]!=NULL){
        switch(j){
@@ -368,6 +377,12 @@
        }
                                                                                
        matchf=open(buffer,O_RDONLY|O_NONBLOCK);
+       for (k = 0; (k<10) && (matchf==-1); k++) { 
+         fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 second.\n", buffer, strerror(errno));
+         usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+         matchf=open(buffer,O_RDONLY|O_NONBLOCK);
+       }
+
        if(matchf!=-1){
          if(get_scsi_id(matchf,&b)==0){
            if(a.bus==b.bus && a.id==b.id && a.lun==b.lun){
@@ -438,7 +453,7 @@
   cdrom_drive *d=NULL;
   struct stat i_st;
   struct stat g_st;
-  int i_fd=-1;
+  int i_fd=-1, i;
   int g_fd=-1;
   int version;
   int type;
@@ -534,8 +549,20 @@
     goto cdda_identify_scsi_fail;
   }
                                                                                
-  if(ioctl_device)i_fd=open(ioctl_device,O_RDONLY|O_NONBLOCK);
-  g_fd=open(generic_device,O_RDWR);
+  if(ioctl_device) {
+    i_fd=open(ioctl_device,O_RDONLY|O_NONBLOCK|O_EXCL);
+    for(i=0; (i<10) && (i_fd==-1); i++) { 
+      fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 second.\n", ioctl_device, strerror(errno));
+      usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+      i_fd=open(ioctl_device,O_RDONLY|O_NONBLOCK|O_EXCL);
+    }
+  }
+  g_fd=open(generic_device,O_RDWR|O_EXCL);
+  for(i=0; (i<10) && (g_fd==-1); i++) { 
+    fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 second.\n", generic_device, strerror(errno));
+    usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+    g_fd=open(generic_device,O_RDWR|O_EXCL);
+  }
                                                                                
   if(ioctl_device && i_fd==-1)
     idperror(messagedest,messages,"\t\tCould not open SCSI cdrom device "
@@ -661,7 +688,7 @@
                                                                                
   cdrom_drive *d=NULL;
   struct stat st;
-  int fd=-1;
+  int fd=-1,i;
                                                                                
   idmessage(messagedest,messages,"\tTesting %s for file/test interface",
            filename);
@@ -678,7 +705,17 @@
     return(NULL);
   }
                                                                                
-  fd=open(filename,O_RDONLY);
+  /* I'm not certain this one nees O_EXCL, but it can't hurt */
+  fd=open(filename,O_RDONLY|O_EXCL);
+  for(i=0; (i<10) && (fd==-1); i++) { 
+    fprintf(stderr, "Error trying to open %s exclusively (%s). retrying in 1 second.\n", filename, strerror(errno));
+    usleep(1000000 + 100000.0 * rand()/(RAND_MAX+1.0));
+    fd=open(filename,O_RDONLY|O_EXCL);
+  }
+
+  if(ioctl_device && i_fd==-1)
+    idperror(messagedest,messages,"\t\tCould not open SCSI cdrom device "
+            "%s (continuing)",ioctl_device);
                                                                                
   if(fd==-1){
     idperror(messagedest,messages,"\t\tCould not open file %s",filename);

<p>Questions?  Comments?  Mild flames?


--
        Peter

"Java is the most distressing thing to happen to computing since MS-DOS."
		-- Alan Kay
--- >8 ----
List archives:  http://www.xiph.org/archives/
Paranoia homepage: http://www.xiph.org/paranoia/
To unsubscribe from this list, send mail to 'paranoia-dev-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.




More information about the Paranoia-dev mailing list