From d7f91539eaeb2be80bf277c57919258d488216ea Mon Sep 17 00:00:00 2001 From: Boian Bonev Date: Tue, 28 Sep 2021 01:37:57 +0300 Subject: [PATCH] mtd: some basic code cleanups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. systemd-commit: 41b9d436b2739cbe8bf9482b665d85d59d06bc0e Author: Lennart Poettering Date: Tue Apr 24 17:50:01 2018 +0200 --- src/mtd_probe/Makefile.am | 6 ++++- src/mtd_probe/mtd_probe.c | 27 +++++++++++------------ src/mtd_probe/mtd_probe.h | 2 +- src/mtd_probe/probe_smartmedia.c | 38 ++++++++++++++++++-------------- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/mtd_probe/Makefile.am b/src/mtd_probe/Makefile.am index 2181733d2..2ee2f30dc 100644 --- a/src/mtd_probe/Makefile.am +++ b/src/mtd_probe/Makefile.am @@ -1,7 +1,7 @@ ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS} AM_CPPFLAGS = \ - -I $(top_srcdir)/src/shared + -I $(top_srcdir)/src/shared udevlibexec_PROGRAMS = \ mtd_probe @@ -10,3 +10,7 @@ mtd_probe_SOURCES = \ mtd_probe.c \ mtd_probe.h \ probe_smartmedia.c + +mtd_probe_LDADD = \ + $(top_builddir)/src/libudev/libudev-private.la \ + $(top_builddir)/src/udev/libudev-core.la diff --git a/src/mtd_probe/mtd_probe.c b/src/mtd_probe/mtd_probe.c index c90283ead..1512f950b 100644 --- a/src/mtd_probe/mtd_probe.c +++ b/src/mtd_probe/mtd_probe.c @@ -31,29 +31,28 @@ #include #include "mtd_probe.h" -int main(int argc, char** argv) -{ - int mtd_fd; - int error; +int main(int argc, char** argv) { + 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/mtd_probe/mtd_probe.h b/src/mtd_probe/mtd_probe.h index caea5c269..1763f6373 100644 --- a/src/mtd_probe/mtd_probe.h +++ b/src/mtd_probe/mtd_probe.h @@ -48,4 +48,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/mtd_probe/probe_smartmedia.c b/src/mtd_probe/probe_smartmedia.c index a007ccee2..91e0c7d85 100644 --- a/src/mtd_probe/probe_smartmedia.c +++ b/src/mtd_probe/probe_smartmedia.c @@ -32,29 +32,32 @@ 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; int spare_count; - char* cis_buffer = malloc(SM_SECTOR_SIZE); + uint8_t *cis_buffer = NULL; int offset; int cis_found = 0; + cis_buffer = malloc(SM_SECTOR_SIZE); if (!cis_buffer) - return; + return log_oom(); - if (info->type != MTD_NANDFLASH) + if (info->type != MTD_NANDFLASH) { + log_debug("Not marked MTD_NANDFLASH."); goto exit; + } sector_size = info->writesize; block_size = info->erasesize; size_in_megs = info->size / (1024 * 1024); - if (sector_size != SM_SECTOR_SIZE && sector_size != SM_SMALL_PAGE) + if (!IN_SET(sector_size, SM_SECTOR_SIZE, SM_SMALL_PAGE)) { + log_debug("Unexpected sector size: %i", sector_size); goto exit; + } switch(size_in_megs) { case 1: @@ -69,27 +72,30 @@ 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); - if (read(mtd_fd, cis_buffer, SM_SECTOR_SIZE) == SM_SECTOR_SIZE){ + 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) + if (!cis_found) { + log_debug("CIS not found"); goto exit; + } if (memcmp(cis_buffer, cis_signature, sizeof(cis_signature)) != 0 && - (memcmp(cis_buffer + SM_SMALL_PAGE, cis_signature, - sizeof(cis_signature)) != 0)) + memcmp(cis_buffer + SM_SMALL_PAGE, cis_signature, sizeof(cis_signature)) != 0) { + log_debug("CIS signature didn't match"); goto exit; + } printf("MTD_FTL=smartmedia\n"); free(cis_buffer); - exit(0); + return 0; exit: free(cis_buffer); - return; + return -1; }