Merge pull request #124 from Moodstocks/ppm-warn

ppm.c: fix fread / fscanf warnings

6 files changed, 35 insertions, 14 deletions

diff --git a/assets/ascii.pgm b/assets/P2.pgm
index 0e76d7d..0e76d7d 100644
--- a/assets/ascii.pgm
+++ b/assets/P2.pgm
diff --git a/assets/P4.pbm b/assets/P4.pbm
new file mode 100644
index 0000000..0cc3736
--- /dev/null
+++ b/assets/P4.pbm
@@ -0,0 +1,3 @@
+P4
+1 1
+€
\ No newline at end of file
diff --git a/assets/test.pgm b/assets/P5.pgm
index b4ea2fb..b4ea2fb 100644
--- a/assets/test.pgm
+++ b/assets/P5.pgm
diff --git a/assets/test.ppm b/assets/P6.ppm
index 68b997c..68b997c 100644
--- a/assets/test.ppm
+++ b/assets/P6.ppm
diff --git a/generic/ppm.c b/generic/ppm.c
index cb1544e..19e62be 100644
--- a/generic/ppm.c
+++ b/generic/ppm.c
@@ -41,35 +41,46 @@ static int libppm_(Main_load)(lua_State *L)
   //printf("Loading PPM\nMAGIC: %c%c\nWidth: %ld, Height: %ld\nChannels: %d, Bits-per-pixel: %d\n", p, n, W, H, D, bps);
 
   // load data
+  int ok = 1;
+  size_t s;
   unsigned char *r = NULL;
   if ( n=='6' ) {
     C = 3;
-    r = (unsigned char *)malloc(W*H*C*bpc);
-    fread ( r, 1, W*H*C*bpc, fp );
+    s = W*H*C*bpc;
+    r = malloc(s);
+    if (fread ( r, 1, s, fp ) < s) ok = 0;
   } else if ( n=='5' ) {
     C = 1;
-    r = (unsigned char *)malloc(W*H*C*bpc);
-    fread ( r, 1, W*H*C*bpc, fp );
+    s = W*H*C*bpc;
+    r = malloc(s);
+    if (fread ( r, 1, s, fp ) < s) ok = 0;
   } else if ( n=='3' ) {
     int c,i;
     C = 3;
-    r = (unsigned char *)malloc(W*H*C);
-    for (i=0; i<W*H*C; i++) {
-      fscanf ( fp, "%d", &c );
+    s = W*H*C;
+    r = malloc(s);
+    for (i=0; i<s; i++) {
+      if (fscanf ( fp, "%d", &c ) != 1) { ok = 0; break; }
       r[i] = 255*c / D;
     }
   } else if ( n=='2' ) {
     int c,i;
     C = 1;
-    r = (unsigned char *)malloc(W*H*C);
-    for (i=0; i<W*H*C; i++) {
-      fscanf ( fp, "%d", &c );
+    s = W*H*C;
+    r = malloc(s);
+    for (i=0; i<s; i++) {
+      if (fscanf ( fp, "%d", &c ) != 1) { ok = 0; break; }
       r[i] = 255*c / D;
     }
   } else {
     W=H=C=0;
     fclose ( fp );
-    luaL_error(L, "corrupted file");
+    luaL_error(L, "unsupported magic number: P%c", n);
+  }
+
+  if (!ok) {
+    fclose ( fp );
+    luaL_error(L, "corrupted file or read error");
   }
 
   // export tensor
diff --git a/test/test.lua b/test/test.lua
index 8af6f7e..5138514 100644
--- a/test/test.lua
+++ b/test/test.lua
@@ -473,7 +473,7 @@ function test.test_ppmload()
     -- This makes possible to implement a non regression test vs. the former
     -- PPM loader which had for effect to skip the first 85 pixels because of
     -- a header parser bug
-    local img = image.load(getTestImagePath("test.ppm"))
+    local img = image.load(getTestImagePath("P6.ppm"))
     local pix = img[{ {}, {1}, {1} }]
 
     -- Check the first pixel is blue
@@ -487,7 +487,7 @@ function test.test_pgmaload()
     -- ascii.ppm is a PGMA file (ascii pgm)
     -- example comes from ehere
     -- http://people.sc.fsu.edu/~jburkardt/data/pgma/pgma.html
-    local img = image.load(getTestImagePath("ascii.pgm"), 1, 'byte')
+    local img = image.load(getTestImagePath("P2.pgm"), 1, 'byte')
     local max_gray = 15 -- 4th line of ascii.pgm
     local ascii_val = 3 -- pixel (2,2) in the file
     local pix_val = math.floor(255 * ascii_val / max_gray)
@@ -505,13 +505,20 @@ function test.test_pgmload()
     -- This makes possible to implement a non regression test vs. the former
     -- PPM loader which had for effect to skip the first 85 pixels because of
     -- a header parser bug
-    local img = image.load(getTestImagePath("test.pgm"))
+    local img = image.load(getTestImagePath("P5.pgm"))
     local pix = img[{ {}, {1}, {1} }]
 
     local ref = torch.zeros(1, 1, 1); ref[1][1][1] = 0.07
     tester:assertTensorEq(pix, ref, 0.001, "PPM load: first pixel check failed")
 end
 
+function test.test_pbmload()
+   -- test.pbm is a Portable BitMap (not supported)
+   local ok, msg = pcall(image.loadPPM, getTestImagePath("P4.pbm"))
+   tester:assert(not ok, "PBM format should not be loaded")
+   tester:assert(string.match(msg, "unsupported magic number"))
+end
+
 
 function image.test(tests, seed)
    seed = seed or os.time()