Package: sng
Version: 1.0.2-5
Severity: normal

SNG exits with a zero (success) status in the presence of at least some
errors.

The attached bad.sng file contains a deliberate error.  To reproduce:

  sng bad.sng && echo 'Success reported for erroneous file'

The comments in the code suggest an awareness of at least part of the
problem, but incorrectly state that it works "if you are only checking
the status of a single file".

Fix: keep track of error statuses throughout, and exit with the greatest
seen.  Patch attached.

-- System Information:
Debian Release: 4.0
  APT prefers stable
  APT policy: (500, 'stable'), (50, 'unstable')
Architecture: amd64 (x86_64)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.18-4-amd64
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)

Versions of packages sng depends on:
ii  libc6                  2.3.6.ds1-13etch5 GNU C Library: Shared libraries
ii  libpng12-0             1.2.15~beta5-1    PNG library - runtime
ii  x11-common             1:7.1.0-19        X Window System (X.Org) infrastruc
ii  zlib1g                 1:1.2.3-13        compression library - runtime

sng recommends no packages.

-- no debconf information

-- 
Aaron Crane ** http://aaroncrane.co.uk/
#SNG: from bad.png
IHDR {
    width: 1; height: 1; bitdepth: 8;
    using brightly coloured machine tools;
}
IMAGE {
    pixels hex
ffffff
}
diff --git sng-1.0.2.orig/main.c sng-1.0.2/main.c
index 21d4697..28577a7 100644
--- sng-1.0.2.orig/main.c
+++ sng-1.0.2/main.c
@@ -9,7 +9,6 @@
 #include "config.h"
 
 int verbose;
-int sng_error;
 int idat;
 
 png_struct *png_ptr;
@@ -152,9 +151,15 @@ void initialize_hash(int hashfunc(color_item *),
  *
  ************************************************************************/
 
+static int max(int x, int y)
+{
+    return x > y ? x : y;
+}
+
 int main(int argc, char *argv[])
 {
     int i = 1;
+    int error_status = 0;
 
 #ifdef __EMX__
     _wildcard(&argc, &argv);   /* Unix-like globbing for OS/2 and DOS */
@@ -213,6 +218,7 @@ int main(int argc, char *argv[])
 	    if (argv[i][dot] != '.')
 	    {
 		fprintf(stderr, "sng: %s is neither SNG nor PNG\n", argv[i]);
+		error_status = max(error_status, 1);
 		continue;
 	    }
 	    else if (strcmp(argv[i] + dot, ".sng") == 0)
@@ -232,6 +238,7 @@ int main(int argc, char *argv[])
 	    else
 	    {
 		fprintf(stderr, "sng: %s is neither SNG nor PNG\n", argv[i]);
+		error_status = max(error_status, 1);
 		continue;
 	    }
 
@@ -243,6 +250,7 @@ int main(int argc, char *argv[])
 		fprintf(stderr,
 			"sng: couldn't open %s for input (%d)\n",
 			argv[i], errno);
+		error_status = max(error_status, 1);
 		continue;
 	    }
 	    if ((fpout = fopen(outfile, "w")) == NULL)
@@ -250,17 +258,16 @@ int main(int argc, char *argv[])
 		fprintf(stderr,
 			"sng: couldn't open for output %s (%d)\n",
 			outfile, errno);
+		error_status = max(error_status, 1);
 		continue;
 	    }
 
 	    if (sng2png)
-		sngc(fpin, argv[i], fpout);
+		error_status = max(error_status, sngc(fpin, argv[i], fpout));
 	    else
-		sngd(fpin, argv[i], fpout);
+		error_status = max(error_status, sngd(fpin, argv[i], fpout));
 	}
     }
 
-    /* This only returns the error on the last file.  Works OK if you are only
-     * checking the status of a single file. */
-    return sng_error;
+    return error_status;
 }
diff --git sng-1.0.2.orig/sng.h sng-1.0.2/sng.h
index 2e9872f..4ea17c8 100644
--- sng-1.0.2.orig/sng.h
+++ sng-1.0.2/sng.h
@@ -25,7 +25,6 @@ extern void initialize_hash(int hashfunc(color_item *),
 
 extern int verbose;
 extern int idat;
-extern int sng_error;
 
 extern int linenum;
 extern char *file;
diff --git sng-1.0.2.orig/sngd.c sng-1.0.2/sngd.c
index 5854a0e..a91fedd 100644
--- sng-1.0.2.orig/sngd.c
+++ sng-1.0.2/sngd.c
@@ -33,6 +33,9 @@ static char *current_file;
 png_structp png_ptr;
 png_infop info_ptr;
 
+/* Error status for the file being processed; reset to 0 at the top of sngd() */
+static int sng_error;
+
 /*****************************************************************************
  *
  * Interface to RGB database
@@ -1061,7 +1064,6 @@ int sngd(FILE *fp, char *name, FILE *fpout)
    if (png_ptr == NULL)
    {
       fclose(fp);
-      sng_error = 1;
       return(1);
    }
 
@@ -1071,8 +1073,7 @@ int sngd(FILE *fp, char *name, FILE *fpout)
    {
       fclose(fp);
       png_destroy_read_struct(&png_ptr, (png_infopp)NULL, (png_infopp)NULL);
-      sng_error = 1;
-      return(FAIL);
+      return 1;
    }
 
    /* Set error handling if you are using the setjmp/longjmp method (this is
@@ -1085,7 +1086,6 @@ int sngd(FILE *fp, char *name, FILE *fpout)
       png_destroy_read_struct(&png_ptr, &info_ptr, (png_infopp)NULL);
       fclose(fp);
       /* If we get here, we had a problem reading the file */
-      sng_error = 1;
       return(1);
    }
 
@@ -1146,8 +1146,8 @@ int sngd(FILE *fp, char *name, FILE *fpout)
    /* close the file */
    fclose(fp);
 
-   /* that's it */
-   return(0);
+   /* that's it; return this file's error status */
+   return sng_error;
 }
 
 /* sngd.c ends here */

Reply via email to