mtd: some basic code cleanups

While looking at our exit() invocations I noticed that the mtd_probe
stuff uses 'exit(-1)' at various places, which is not really a good
idea, as exit codes of processes on Linux are supposed to be in the
range of 0…255.

This patch cleans that up a bit, and fixes a number of other things:

1. Let's always let main() exit, nothing intermediary. We generally
   don't like code that invokes exit() on its own.

2. Close the file descriptors opened.

3. Some logging for errors is added, mostly on debug level.

Please review this with extra care. As I don't have the right hardware
to test this patch I only did superficial testing.

diff --git a/src/udev/mtd_probe/mtd_probe.c b/src/udev/mtd_probe/mtd_probe.c
index 1d692f1187abe434a9d39385de54e9ce666f8bfd..fa5f41faa5aa0909d1396be5abc5a4c5892cc81e 100644 (file)
--- a/src/udev/mtd_probe/mtd_probe.c
+++ b/src/udev/mtd_probe/mtd_probe.c
@@ -18,6 +18,7 @@
  * Boston, MA  02110-1301  USA
  */
 
+#include <errno.h>
 #include <fcntl.h>
 #include <mtd/mtd-user.h>
 #include <stdio.h>
@@ -27,31 +28,32 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "alloc-util.h"
+#include "fd-util.h"
 #include "mtd_probe.h"
 
-int main(int argc, char** argv)
-{
-        int mtd_fd;
-        int error;
+int main(int argc, char** argv) {
+        _cleanup_close_ int mtd_fd = -1;
         mtd_info_t mtd_info;
 
         if (argc != 2) {
                 printf("usage: mtd_probe /dev/mtd[n]\n");
-                return 1;
+                return EXIT_FAILURE;
         }
 
         mtd_fd = open(argv[1], O_RDONLY|O_CLOEXEC);
-        if (mtd_fd == -1) {
-                perror("open");
-                exit(-1);
+        if (mtd_fd < 0) {
+                log_error_errno(errno, "Failed to open: %m");
+                return EXIT_FAILURE;
         }
 
-        error = ioctl(mtd_fd, MEMGETINFO, &mtd_info);
-        if (error == -1) {
-                perror("ioctl");
-                exit(-1);
+        if (ioctl(mtd_fd, MEMGETINFO, &mtd_info) < 0) {
+                log_error_errno(errno, "Failed to issue MEMGETINFO ioctl: %m");
+                return EXIT_FAILURE;
         }
 
-        probe_smart_media(mtd_fd, &mtd_info);
-        return -1;
+        if (probe_smart_media(mtd_fd, &mtd_info) < 0)
+                return EXIT_FAILURE;
+
+        return EXIT_SUCCESS;
 }

diff --git a/src/udev/mtd_probe/mtd_probe.h b/src/udev/mtd_probe/mtd_probe.h
index 39841f9b7d643edf03f16ea448090c6a51a44038..20ff862e51d1d229696853313a586a6df2ce4258 100644 (file)
--- a/src/udev/mtd_probe/mtd_probe.h
+++ b/src/udev/mtd_probe/mtd_probe.h
@@ -49,4 +49,4 @@ struct sm_oob {
 #define SM_SMALL_PAGE                 256
 #define SM_SMALL_OOB_SIZE        8
 
-void probe_smart_media(int mtd_fd, mtd_info_t *info);
+int probe_smart_media(int mtd_fd, mtd_info_t *info);

diff --git a/src/udev/mtd_probe/probe_smartmedia.c b/src/udev/mtd_probe/probe_smartmedia.c
index c058d83c2ed4da1def5fae5fc3a63f301300dfa0..da937a399dc6183944436c3cca695c1aa286ad88 100644 (file)
--- a/src/udev/mtd_probe/probe_smartmedia.c
+++ b/src/udev/mtd_probe/probe_smartmedia.c
@@ -18,6 +18,7 @@
  * Boston, MA  02110-1301  USA
  */
 
+#include <errno.h>
 #include <fcntl.h>
 #include <mtd/mtd-user.h>
 #include <stdint.h>
@@ -35,7 +36,7 @@ static const uint8_t cis_signature[] = {
         0x01, 0x03, 0xD9, 0x01, 0xFF, 0x18, 0x02, 0xDF, 0x01, 0x20
 };
 
-void probe_smart_media(int mtd_fd, mtd_info_t* info) {
+int probe_smart_media(int mtd_fd, mtd_info_t* info) {
         int sector_size;
         int block_size;
         int size_in_megs;
@@ -46,17 +47,21 @@ void probe_smart_media(int mtd_fd, mtd_info_t* info) {
 
         cis_buffer = malloc(SM_SECTOR_SIZE);
         if (!cis_buffer)
-                return;
+                return log_oom();
 
-        if (info->type != MTD_NANDFLASH)
-                goto exit;
+        if (info->type != MTD_NANDFLASH) {
+                log_debug("Not marked MTD_NANDFLASH.");
+                return -EINVAL;
+        }
 
         sector_size = info->writesize;
         block_size = info->erasesize;
         size_in_megs = info->size / (1024 * 1024);
 
-        if (!IN_SET(sector_size, SM_SECTOR_SIZE, SM_SMALL_PAGE))
-                goto exit;
+        if (!IN_SET(sector_size, SM_SECTOR_SIZE, SM_SMALL_PAGE)) {
+                log_debug("Unexpected sector size: %i", sector_size);
+                return -EINVAL;
+        }
 
         switch(size_in_megs) {
         case 1:
@@ -71,26 +76,26 @@ void probe_smart_media(int mtd_fd, mtd_info_t* info) {
                 break;
         }
 
-        for (offset = 0 ; offset < block_size * spare_count ;
-                                                offset += sector_size) {
-                lseek(mtd_fd, SEEK_SET, offset);
+        for (offset = 0; offset < block_size * spare_count; offset += sector_size) {
+                (void) lseek(mtd_fd, SEEK_SET, offset);
+
                 if (read(mtd_fd, cis_buffer, SM_SECTOR_SIZE) == SM_SECTOR_SIZE) {
                         cis_found = 1;
                         break;
                 }
         }
 
-        if (!cis_found)
-                goto exit;
+        if (!cis_found) {
+                log_debug("CIS not found");
+                return -EINVAL;
+        }
 
         if (memcmp(cis_buffer, cis_signature, sizeof(cis_signature)) != 0 &&
-                (memcmp(cis_buffer + SM_SMALL_PAGE, cis_signature,
-                        sizeof(cis_signature)) != 0))
-                goto exit;
+            memcmp(cis_buffer + SM_SMALL_PAGE, cis_signature, sizeof(cis_signature)) != 0) {
+                log_debug("CIS signature didn't match");
+                return -EINVAL;
+        }
 
         printf("MTD_FTL=smartmedia\n");
-        exit(EXIT_SUCCESS);
-
-exit:
-        return;
+        return 0;
 }