All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC
@ 2017-02-02 22:37 Todor Minchev
  2017-02-02 22:37 ` [PATCH 1/3] Makefile: add verbosity and debug options to Makefile Todor Minchev
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Todor Minchev @ 2017-02-02 22:37 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

This patchset adds database extraction and fingerprint quering
functionality to RMC

Example:

Output fingerprint contents to terminal:

./rmc -F -i rmc.fingerprint

Extract RMC database:

./rmc -E -d rmc.db

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10092

Todor Minchev (3):
  Makefile: add verbosity and debug options to Makefile
  rmc: Enable reading the contents of an existing fingerprint file
  rmc: add database extraction functionality

 Makefile              |  31 ++++++----
 inc/rmc_api.h         |   9 +++
 src/lib/api.c         |  85 ++++++++++++++++++++++++++-
 src/lib/common/rmcl.c |   3 +-
 src/rmc.c             | 157 +++++++++++++++++++++++++++++++++-----------------
 5 files changed, 219 insertions(+), 66 deletions(-)

-- 
2.11.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] Makefile: add verbosity and debug options to Makefile
  2017-02-02 22:37 [PATCH 0/3] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
@ 2017-02-02 22:37 ` Todor Minchev
  2017-02-06 19:06   ` Jianxun Zhang
  2017-02-02 22:37 ` [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
  2017-02-02 22:37 ` [PATCH 3/3] rmc: add database extraction functionality Todor Minchev
  2 siblings, 1 reply; 11+ messages in thread
From: Todor Minchev @ 2017-02-02 22:37 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

By default Makefile verbosity is disabled (V=0). Verbosity can be enabled by
setting the V environment variable to any value not equal to 0 (e.g V=1)

Example:
make clean V=1; make V=1

A debug version of the rmc binary can be built by using the debug
Makefile target. This will include debug symbols and will disable compiler
optimizations when compiling rmc.

Example:

make debug

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 Makefile | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 9ade775..d85d8e9 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,12 @@
 # Copyright (C) 2016 Jianxun Zhang <jianxun.zhang@intel.com>
 
+V ?= 0
+ifeq ($(V),0)
+  VERBOSITY = @
+else
+  VERBOSITY =
+endif
+
 TOPDIR = $(shell if [ -z "$$PWD" ]; then pwd; else echo "$$PWD"; fi)
 
 RMC_TOOL_SRC := $(wildcard src/*.c)
@@ -20,28 +27,32 @@ RMC_INSTALL_HEADER_PATH := $(RMC_INSTALL_PREFIX)/include/rmc/
 
 ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
 
+
 RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
 
 all: rmc
+debug: RMC_CFLAGS += -DDEBUG -g -O0
+debug: rmc
 
 $(ALL_OBJS): %.o: %.c
-	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
+	$(VERBOSITY)$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
 
 librmc: $(RMC_LIB_OBJ)
-	@$(AR) rcs src/lib/$@.a $^
+	$(VERBOSITY)$(AR) rcs src/lib/$@.a $^
 
 rmc: $(RMC_TOOL_OBJ) librmc
-	@$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) src/lib/librmc.a -o src/$@
+	$(VERBOSITY)$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) \
+  src/lib/librmc.a -o src/$@
 
 clean:
-	@rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
+	$(VERBOSITY)rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
 
 .PHONY: clean rmc librmc
 
 install:
-	@mkdir -p $(RMC_INSTALL_BIN_PATH)
-	@install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
-	@mkdir -p $(RMC_INSTALL_LIB_PATH)
-	@install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
-	@mkdir -p $(RMC_INSTALL_HEADER_PATH)
-	@install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
+	$(VERBOSITY)mkdir -p $(RMC_INSTALL_BIN_PATH)
+	$(VERBOSITY)install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
+	$(VERBOSITY)mkdir -p $(RMC_INSTALL_LIB_PATH)
+	$(VERBOSITY)install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
+	$(VERBOSITY)mkdir -p $(RMC_INSTALL_HEADER_PATH)
+	$(VERBOSITY)install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file
  2017-02-02 22:37 [PATCH 0/3] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
  2017-02-02 22:37 ` [PATCH 1/3] Makefile: add verbosity and debug options to Makefile Todor Minchev
@ 2017-02-02 22:37 ` Todor Minchev
  2017-02-06 20:01   ` Jianxun Zhang
  2017-02-02 22:37 ` [PATCH 3/3] rmc: add database extraction functionality Todor Minchev
  2 siblings, 1 reply; 11+ messages in thread
From: Todor Minchev @ 2017-02-02 22:37 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

The contents of an existing fingerprint file can be read and output on
the command line with the following options:

./rmc -F -i input_fingerprint_file

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 src/rmc.c | 121 +++++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 76 insertions(+), 45 deletions(-)

diff --git a/src/rmc.c b/src/rmc.c
index 062dd36..a051ccf 100644
--- a/src/rmc.c
+++ b/src/rmc.c
@@ -14,33 +14,35 @@
 #include <rmc_api.h>
 
 #define USAGE "RMC (Runtime Machine configuration) Tool\n" \
-    "NOTE: Most of usages require root permission (sudo)\n" \
-    "rmc -F [-o output_fingerprint]\n" \
+    "NOTE: Most of usages require root permission (sudo)\n\n" \
+    "rmc -F [-o output_fingerprint] | -i input_fingerprint\n" \
     "rmc -R [-f <fingerprint file>] -b <blob file list> [-o output_record]\n" \
     "rmc -D <rmc record file list> [-o output_database]\n" \
-	"rmc -B <name of file blob> -d <rmc database file> -o output_file\n" \
-	"\n" \
-	"-F: generate board rmc fingerprint of board\n" \
-	"-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
-    "-f: fingerprint file to be packed in record, rmc will create a fingerprint for board and use it internally to\n" \
-    "    generate record if -f is missed.\n" \
-    "-b: files to be packed in record\n" \
-	"-G: generate rmc database file with records specified in record file list\n" \
-	"-B: get a flie blob with specified name associated to the board rmc is running on\n" \
-	"-d: database file to be queried\n" \
-	"-o: path and name of output file of a specific command\n" \
-	"\n" \
-    "Examples (Steps in an order to add board support into rmc):\n" \
-    "generate board fingerprint:\n" \
-    "rmc -F\n\n" \
-    "generate a rmc record for the board with two file blobs, output to:\n" \
-    "a specified file:\n" \
-    "rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
-    "generate a rmc database file with records from 3 different boards:\n" \
-    "rmc -D board1_record board2_record board3_record\n\n" \
-    "query a file blob named audio.conf associated to the board rmc is running on in database my_rmc.db and output\n" \
-    "to /tmp/new_audio.conf:\n" \
-    "rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
+    "rmc -B <name of file blob> -d <rmc database file> -o output_file\n\n" \
+  "-F: manage fingerprint file\n" \
+    "\t-o output_file: store RMC fingerprint of current board in output_file\n" \
+    "\t-i input_file: print RMC fingerprint stored in input_file\n\n" \
+  "-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
+    "\t-f intput_file : input fingerprint file to be packed in record\n\n" \
+    "\tNOTE: RMC will create a fingerprint for the board and use it to\n" \
+    "\tgenerate record if an input fingerprint file is not provided.\n\n" \
+    "\t-b: files to be packed in record\n\n" \
+  "-G: generate rmc database file with records specified in record file list\n\n" \
+  "-B: get a file blob with specified name associated to the board rmc is\n" \
+  "running on\n" \
+    "\t-d: database file to be queried\n" \
+    "\t-o: path and name of output file of a specific command\n\n" \
+    "Examples (Steps in an order to add board support into rmc):\n\n" \
+    "1. Generate board fingerprint:\n" \
+    "\t./rmc -F\n\n" \
+    "2. Generate a rmc record for the board with two file blobs and save it\n" \
+    "to a specified file:\n" \
+    "\t./rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
+    "3. Generate a rmc database file with records from 3 different boards:\n" \
+    "\t./rmc -D board1_record board2_record board3_record\n\n" \
+    "4. Query a file blob named audio.conf associated to the board rmc is\n" \
+    "running on in database my_rmc.db and output to /tmp/new_audio.conf:\n" \
+    "\t./rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
 
 
 #define RMC_OPT_CAP_F   (1 << 0)
@@ -51,6 +53,7 @@
 #define RMC_OPT_O       (1 << 5)
 #define RMC_OPT_B       (1 << 6)
 #define RMC_OPT_D       (1 << 7)
+#define RMC_OPT_I       (1 << 8)
 
 static void usage () {
     fprintf(stdout, USAGE);
@@ -78,7 +81,7 @@ static void dump_fingerprint(rmc_fingerprint_t *fp) {
 static int write_fingerprint_file(const char* pathname, rmc_fingerprint_t *fp) {
     int i;
     int first = 0;
-
+    /* TODO - do we need to open/close file multiple times to write each field */
     for (i = 0; i < RMC_FINGER_NUM; i++) {
         if (write_file(pathname, &fp->rmc_fingers[i].type, sizeof(fp->rmc_fingers[i].type), first))
             return 1;
@@ -214,7 +217,6 @@ read_fp_done:
 static rmc_file_t *read_policy_file(char *pathname, int type) {
     rmc_file_t *tmp = NULL;
     rmc_size_t policy_len = 0;
-    int ret;
     char *path_token;
 
     if ((tmp = calloc(1, sizeof(rmc_file_t))) == NULL) {
@@ -226,8 +228,7 @@ static rmc_file_t *read_policy_file(char *pathname, int type) {
     tmp->next = NULL;
 
     if (type == RMC_GENERIC_FILE) {
-        ret = read_file(pathname, (char **)&tmp->blob, &policy_len);
-        if (ret) {
+        if (read_file(pathname, (char **)&tmp->blob, &policy_len)) {
             fprintf(stderr, "Failed to read file %s\n\n", pathname);
             free(tmp);
             return NULL;
@@ -311,7 +312,7 @@ int main(int argc, char **argv){
     /* parse options */
     opterr = 0;
 
-    while ((c = getopt(argc, argv, "FRD:B:b:f:o:d:")) != -1)
+    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
         switch (c) {
         case 'F':
             options |= RMC_OPT_CAP_F;
@@ -352,6 +353,10 @@ int main(int argc, char **argv){
             output_path = optarg;
             options |= RMC_OPT_O;
             break;
+        case 'i':
+            input_fingerprint = optarg;
+            options |= RMC_OPT_I;
+            break;
         case 'f':
             input_fingerprint = optarg;
             options |= RMC_OPT_F;
@@ -388,7 +393,8 @@ int main(int argc, char **argv){
             break;
         case '?':
             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
-                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd')
+                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
+                    || optopt == 'i')
                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
             else if (isprint(optopt))
                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
@@ -414,6 +420,15 @@ int main(int argc, char **argv){
         }
     }
 
+    /* sanity check for -i */
+    if (options & RMC_OPT_I) {
+        if (!(options & RMC_OPT_CAP_F)) {
+            fprintf(stderr, "\nWRONG: Option -i cannot be applied without -F\n\n");
+            usage();
+            return 1;
+        }
+    }
+
     /* sanity check for -R */
     if ((options & RMC_OPT_CAP_R) && (!(options & RMC_OPT_B))) {
         fprintf(stderr, "\nWRONG: -b is required when -R is present\n\n");
@@ -563,25 +578,41 @@ int main(int argc, char **argv){
     }
 
     if (options & RMC_OPT_CAP_F) {
-        /* set a default fingerprint file name if user didn't provide one */
-        if (!output_path)
-            output_path = "rmc.fingerprint";
+        /* print fingerpring file to console*/
+        if (options & RMC_OPT_I) {
+            rmc_fingerprint_t fp;
+            /* read fingerprint file*/
+            if (input_fingerprint != NULL) {
+                if (read_fingerprint_from_file(input_fingerprint, &fp, &raw_fp)) {
+                    fprintf(stderr, "Cannot read fingerprint from %s\n\n",
+                    input_fingerprint);
+                    goto main_free;
+                }
+                printf("Successfully read fingerprint from %s \n", input_fingerprint);
+                dump_fingerprint(&fp);
+            }else {
+                printf("Fingerprint file not provided! Exiting.\n");
+            }
+        } else { /* generate fingerprint file for the current board*/
+            /* set a default fingerprint file name if user didn't provide one */
+            if (!output_path)
+                output_path = "rmc.fingerprint";
 
-        if (rmc_get_fingerprint(&fingerprint)) {
-            fprintf(stderr, "Cannot get board fingerprint\n");
-            goto main_free;
-        }
+            if (rmc_get_fingerprint(&fingerprint)) {
+                fprintf(stderr, "Cannot get board fingerprint\n");
+                goto main_free;
+            }
 
-        printf("Got Fingerprint for board:\n\n");
-        dump_fingerprint(&fingerprint);
+            printf("Got Fingerprint for board:\n\n");
+            dump_fingerprint(&fingerprint);
 
-        if (write_fingerprint_file(output_path, &fingerprint)) {
-            fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
+            if (write_fingerprint_file(output_path, &fingerprint)) {
+                fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
+                rmc_free_fingerprint(&fingerprint);
+                goto main_free;
+            }
             rmc_free_fingerprint(&fingerprint);
-            goto main_free;
         }
-
-        rmc_free_fingerprint(&fingerprint);
     }
 
     ret = 0;
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] rmc: add database extraction functionality
  2017-02-02 22:37 [PATCH 0/3] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
  2017-02-02 22:37 ` [PATCH 1/3] Makefile: add verbosity and debug options to Makefile Todor Minchev
  2017-02-02 22:37 ` [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
@ 2017-02-02 22:37 ` Todor Minchev
  2017-02-06 21:09   ` Jianxun Zhang
  2 siblings, 1 reply; 11+ messages in thread
From: Todor Minchev @ 2017-02-02 22:37 UTC (permalink / raw)
  To: yocto, meta-intel, jianxun.zhang; +Cc: Todor Minchev

The contents of an existing database file can be extracted in the
current working directory with the -E option. The top level of the
directory tree is rmc_db_dump and all files corresponding to
a given record will be saved in a separate sub-directory. The sub-directory
name of each record is the signature corresponding to the fingerprint for
that record.

Example:
./src/rmc -E -d rmc.db

Successfully extracted rmc.db

Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
---
 inc/rmc_api.h         |  9 ++++++
 src/lib/api.c         | 85 +++++++++++++++++++++++++++++++++++++++++++++++++--
 src/lib/common/rmcl.c |  3 +-
 src/rmc.c             | 44 +++++++++++++++++++-------
 4 files changed, 126 insertions(+), 15 deletions(-)

diff --git a/inc/rmc_api.h b/inc/rmc_api.h
index a484389..ce26220 100644
--- a/inc/rmc_api.h
+++ b/inc/rmc_api.h
@@ -74,6 +74,15 @@ extern int rmc_query_file_by_fp(rmc_fingerprint_t *fp, char *db_pathname, char *
  */
 extern int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file);
 
+
+/* extract the contents of a database file and store the files corresponding to
+ * each record in a separate directory. The name of each directory is the signature
+ * of the fingerpring for that record
+ * (in) db_pathname: The path and file name of a RMC database file generated by RMC tool
+ * return: 0 on success, non-zero on failure.
+ */
+int dump_db(char *db_pathname) ;
+
 /* 1.3 - Helper APIs */
 
 /* Free allocated data referred in a fingerprint
diff --git a/src/lib/api.c b/src/lib/api.c
index 0adb390..aca8d99 100644
--- a/src/lib/api.c
+++ b/src/lib/api.c
@@ -3,6 +3,7 @@
  * RMC API implementation for Linux user space
  */
 
+#define _GNU_SOURCE
 #include <stdio.h>
 #include <unistd.h>
 #include <errno.h>
@@ -14,8 +15,11 @@
 #include <rmcl.h>
 #include <rsmp.h>
 
-#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
-#define SYSTAB_LEN 4096         /* assume 4kb is enough...*/
+#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
+#define SYSTAB_LEN       4096             /* assume 4kb is enough...*/
+#define DB_DUMP_DIR      "./rmc_db_dump"  /* directory to store db data dump */
+
+extern const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN];
 
 int read_file(const char *pathname, char **data, rmc_size_t* len) {
     int fd = -1;
@@ -325,3 +329,80 @@ int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file) {
 
     return ret;
 }
+
+/*
+ * Dump contents of database file
+ * (in) rmc_db - input database file to extract
+ */
+int dump_db(char *db_pathname) {
+    rmc_meta_header_t meta_header;
+    rmc_db_header_t *db_header = NULL;
+    rmc_record_header_t record_header;
+    rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
+    rmc_uint64_t meta_idx = 0;     /* offset of each meta in a record */
+    rmc_uint64_t file_idx = 0;     /* offset of file in a meta */
+    rmc_file_t file;
+    char *out_dir = NULL, *out_name = NULL;
+    rmc_size_t db_len = 0;
+    rmc_uint8_t *rmc_db = NULL;
+    struct stat s = {0};
+
+    if (read_file(db_pathname, (char **)&rmc_db, &db_len)) {
+        fprintf(stderr, "Failed to read database file\n\n");
+        return 1;
+    }
+
+    db_header = (rmc_db_header_t *)rmc_db;
+
+    /* sanity check of db */
+    if (strncmp((const char *)db_header->signature,
+        (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
+        return 1;
+
+    /* create the top level directory */
+    if (stat(DB_DUMP_DIR, &s) == -1) {
+        if(mkdir(DB_DUMP_DIR, 0755)) {
+            fprintf(stderr, "Failed to create %s directory\n\n", out_name);
+        }
+    }
+
+    /* query the meta. idx: start of record */
+    record_idx = sizeof(rmc_db_header_t);
+    while (record_idx < db_header->length) {
+        memcpy(&record_header, rmc_db + record_idx,
+            sizeof(rmc_record_header_t));
+
+        /* directory name is fingerprint signature */
+        asprintf(&out_dir, "%s/%s/", DB_DUMP_DIR, record_header.signature.raw);
+        if (stat(out_dir, &s) == -1) {
+            if(mkdir(out_dir, 0755)) {
+                fprintf(stderr, "Failed to create %s directory\n\n", out_name);
+            }
+        }
+
+        /* find meta */
+        for (meta_idx = record_idx + sizeof(rmc_record_header_t);
+            meta_idx < record_idx + record_header.length;) {
+            memcpy(&meta_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
+            file_idx = meta_idx + sizeof(rmc_meta_header_t);
+            rmc_ssize_t name_len = strlen((char *)&rmc_db[file_idx]) + 1;
+            file.blob = &rmc_db[file_idx + name_len];
+            file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
+                name_len;
+            file.next = NULL;
+            file.type = RMC_GENERIC_FILE;
+            asprintf(&out_name, "%s%s", out_dir, (char *)&rmc_db[file_idx]);
+            /* write file to dump directory */
+            if (write_file((const char *)out_name, file.blob, file.blob_len, 0))
+                return 1;
+
+            /* next meta */
+            meta_idx += meta_header.length;
+            free(out_name);
+        }
+        /* next record */
+        record_idx += record_header.length;
+        free(out_dir);
+    }
+    return 0;
+}
diff --git a/src/lib/common/rmcl.c b/src/lib/common/rmcl.c
index 67622a0..c996577 100644
--- a/src/lib/common/rmcl.c
+++ b/src/lib/common/rmcl.c
@@ -10,7 +10,7 @@
 #include <rmc_util.h>
 #endif
 
-static const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN] = {'R', 'M', 'C', 'D', 'B'};
+const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN] = {'R', 'M', 'C', 'D', 'B'};
 
 /* compute a finger to signature which is stored in record
  * (in) fingerprint : of board, usually generated by rmc tool and rsmp
@@ -242,7 +242,6 @@ int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rm
                         policy->blob_len = meta_header.length - sizeof(rmc_meta_header_t) - cmd_name_len;
                         policy->next = NULL;
                         policy->type = type;
-
                         return 0;
                     }
                 }
diff --git a/src/rmc.c b/src/rmc.c
index a051ccf..888cbdb 100644
--- a/src/rmc.c
+++ b/src/rmc.c
@@ -32,6 +32,8 @@
   "running on\n" \
     "\t-d: database file to be queried\n" \
     "\t-o: path and name of output file of a specific command\n\n" \
+  "-E: Extract database data to current working directory\n" \
+    "\t-d: database file to extract\n\n" \
     "Examples (Steps in an order to add board support into rmc):\n\n" \
     "1. Generate board fingerprint:\n" \
     "\t./rmc -F\n\n" \
@@ -49,11 +51,12 @@
 #define RMC_OPT_CAP_R   (1 << 1)
 #define RMC_OPT_CAP_D   (1 << 2)
 #define RMC_OPT_CAP_B   (1 << 3)
-#define RMC_OPT_F       (1 << 4)
-#define RMC_OPT_O       (1 << 5)
-#define RMC_OPT_B       (1 << 6)
-#define RMC_OPT_D       (1 << 7)
-#define RMC_OPT_I       (1 << 8)
+#define RMC_OPT_CAP_E   (1 << 4)
+#define RMC_OPT_F       (1 << 5)
+#define RMC_OPT_O       (1 << 6)
+#define RMC_OPT_B       (1 << 7)
+#define RMC_OPT_D       (1 << 8)
+#define RMC_OPT_I       (1 << 9)
 
 static void usage () {
     fprintf(stdout, USAGE);
@@ -312,7 +315,7 @@ int main(int argc, char **argv){
     /* parse options */
     opterr = 0;
 
-    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
+    while ((c = getopt(argc, argv, "FRED:B:b:f:o:i:d:")) != -1)
         switch (c) {
         case 'F':
             options |= RMC_OPT_CAP_F;
@@ -320,6 +323,9 @@ int main(int argc, char **argv){
         case 'R':
             options |= RMC_OPT_CAP_R;
             break;
+        case 'E':
+            options |= RMC_OPT_CAP_E;
+            break;
         case 'D':
             /* we don't know number of arguments for this option at this point,
              * allocate array with argc which is bigger than needed. But we also
@@ -393,8 +399,8 @@ int main(int argc, char **argv){
             break;
         case '?':
             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
-                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
-                    || optopt == 'i')
+                    optopt == 'E' ||  optopt == 'b' || optopt == 'f' || \
+                    optopt == 'o' || optopt == 'd' || optopt == 'i')
                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
             else if (isprint(optopt))
                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
@@ -436,6 +442,13 @@ int main(int argc, char **argv){
         return 1;
     }
 
+    /* sanity check for -E */
+    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_D))) {
+        fprintf(stderr, "\nERROR: -E requires -d <database file name>\n\n");
+        usage();
+        return 1;
+    }
+
     /* sanity check for -B */
     if ((options & RMC_OPT_CAP_B) && (!(options & RMC_OPT_D) || !(options & RMC_OPT_O))) {
         fprintf(stderr, "\nWRONG: -B requires -d and -o\n\n");
@@ -448,7 +461,8 @@ int main(int argc, char **argv){
         rmc_file_t file;
 
         if (!output_path) {
-            fprintf(stderr, "-B internal error, with -o but no output pathname specified\n\n");
+            fprintf(stderr, "-B internal error, with -o but no output \
+                pathname specified\n\n");
             goto main_free;
         }
 
@@ -456,14 +470,22 @@ int main(int argc, char **argv){
             goto main_free;
 
         if (write_file(output_path, file.blob, file.blob_len, 0)) {
-            fprintf(stderr, "-B failed to write file %s to %s\n\n", input_blob_name, output_path);
+            fprintf(stderr, "-B failed to write file %s to %s\n\n",
+                input_blob_name, output_path);
             rmc_free_file(&file);
             goto main_free;
         }
-
         rmc_free_file(&file);
     }
 
+    /* dump database data */
+    if (options & RMC_OPT_CAP_E) {
+        if(dump_db(input_db_path_d))
+            fprintf(stderr, "\nFailed to extract %s\n\n", input_db_path_d);
+        else
+            printf("\nSuccessfully extracted %s\n\n", input_db_path_d);
+    }
+
     /* generate RMC database file */
     if (options & RMC_OPT_CAP_D) {
         int record_idx = 0;
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] Makefile: add verbosity and debug options to Makefile
  2017-02-02 22:37 ` [PATCH 1/3] Makefile: add verbosity and debug options to Makefile Todor Minchev
@ 2017-02-06 19:06   ` Jianxun Zhang
  2017-02-06 22:21     ` Todor Minchev
  0 siblings, 1 reply; 11+ messages in thread
From: Jianxun Zhang @ 2017-02-06 19:06 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto

Todor,
Please refer to my 2 inline comments.

> On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> By default Makefile verbosity is disabled (V=0). Verbosity can be enabled by
> setting the V environment variable to any value not equal to 0 (e.g V=1)
> 
> Example:
> make clean V=1; make V=1
> 
> A debug version of the rmc binary can be built by using the debug
> Makefile target. This will include debug symbols and will disable compiler
> optimizations when compiling rmc.
> 
> Example:
> 
> make debug
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> Makefile | 31 +++++++++++++++++++++----------
> 1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9ade775..d85d8e9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,5 +1,12 @@
> # Copyright (C) 2016 Jianxun Zhang <jianxun.zhang@intel.com>
> 
> +V ?= 0
> +ifeq ($(V),0)
> +  VERBOSITY = @
> +else
> +  VERBOSITY =
> +endif
> +
I am thinking maybe we should remove ‘@‘ in rules and use -s option of make (in recipe) when we want to mute the output. This should reach the same effect without bothering new variables.

We still enable/disable output as a whole anyway.

Let me know if this proposal works for you.

> TOPDIR = $(shell if [ -z "$$PWD" ]; then pwd; else echo "$$PWD"; fi)
> 
> RMC_TOOL_SRC := $(wildcard src/*.c)
> @@ -20,28 +27,32 @@ RMC_INSTALL_HEADER_PATH := $(RMC_INSTALL_PREFIX)/include/rmc/
> 
> ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
> 
> +
> RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
> 
> all: rmc
> +debug: RMC_CFLAGS += -DDEBUG -g -O0
> +debug: rmc
I am not sure if this is necessary because we already have CFLAGS. I think you can reach the same effect without adding a new debug target:
make CFLAGS='-DDEBUG -g -O0’

also refer to commit e8b48e6 in rmc project.

> 
> $(ALL_OBJS): %.o: %.c
> -	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> +	$(VERBOSITY)$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> 
> librmc: $(RMC_LIB_OBJ)
> -	@$(AR) rcs src/lib/$@.a $^
> +	$(VERBOSITY)$(AR) rcs src/lib/$@.a $^
> 
> rmc: $(RMC_TOOL_OBJ) librmc
> -	@$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) src/lib/librmc.a -o src/$@
> +	$(VERBOSITY)$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) \
> +  src/lib/librmc.a -o src/$@
> 
> clean:
> -	@rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> +	$(VERBOSITY)rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> 
> .PHONY: clean rmc librmc
> 
> install:
> -	@mkdir -p $(RMC_INSTALL_BIN_PATH)
> -	@install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> -	@mkdir -p $(RMC_INSTALL_LIB_PATH)
> -	@install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> -	@mkdir -p $(RMC_INSTALL_HEADER_PATH)
> -	@install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_BIN_PATH)
> +	$(VERBOSITY)install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_LIB_PATH)
> +	$(VERBOSITY)install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_HEADER_PATH)
> +	$(VERBOSITY)install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> -- 
> 2.11.0
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file
  2017-02-02 22:37 ` [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
@ 2017-02-06 20:01   ` Jianxun Zhang
  2017-02-06 22:28     ` Todor Minchev
  0 siblings, 1 reply; 11+ messages in thread
From: Jianxun Zhang @ 2017-02-06 20:01 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto

Tudor,
Please refer to my 3 inline comments.

> On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> The contents of an existing fingerprint file can be read and output on
> the command line with the following options:
> 
> ./rmc -F -i input_fingerprint_file
Suggest we have a new top option for dumping in parallel with -F to keep usages clear and simple for users.

> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> src/rmc.c | 121 +++++++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 76 insertions(+), 45 deletions(-)
> 
> diff --git a/src/rmc.c b/src/rmc.c
> index 062dd36..a051ccf 100644
> --- a/src/rmc.c
> +++ b/src/rmc.c
> @@ -14,33 +14,35 @@
> #include <rmc_api.h>
> 
> #define USAGE "RMC (Runtime Machine configuration) Tool\n" \
> -    "NOTE: Most of usages require root permission (sudo)\n" \
> -    "rmc -F [-o output_fingerprint]\n" \
> +    "NOTE: Most of usages require root permission (sudo)\n\n" \
> +    "rmc -F [-o output_fingerprint] | -i input_fingerprint\n" \
>     "rmc -R [-f <fingerprint file>] -b <blob file list> [-o output_record]\n" \
>     "rmc -D <rmc record file list> [-o output_database]\n" \
> -	"rmc -B <name of file blob> -d <rmc database file> -o output_file\n" \
> -	"\n" \
> -	"-F: generate board rmc fingerprint of board\n" \
> -	"-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
> -    "-f: fingerprint file to be packed in record, rmc will create a fingerprint for board and use it internally to\n" \
> -    "    generate record if -f is missed.\n" \
> -    "-b: files to be packed in record\n" \
> -	"-G: generate rmc database file with records specified in record file list\n" \
> -	"-B: get a flie blob with specified name associated to the board rmc is running on\n" \
> -	"-d: database file to be queried\n" \
> -	"-o: path and name of output file of a specific command\n" \
> -	"\n" \
> -    "Examples (Steps in an order to add board support into rmc):\n" \
> -    "generate board fingerprint:\n" \
> -    "rmc -F\n\n" \
> -    "generate a rmc record for the board with two file blobs, output to:\n" \
> -    "a specified file:\n" \
> -    "rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
> -    "generate a rmc database file with records from 3 different boards:\n" \
> -    "rmc -D board1_record board2_record board3_record\n\n" \
> -    "query a file blob named audio.conf associated to the board rmc is running on in database my_rmc.db and output\n" \
> -    "to /tmp/new_audio.conf:\n" \
> -    "rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
> +    "rmc -B <name of file blob> -d <rmc database file> -o output_file\n\n" \
> +  "-F: manage fingerprint file\n" \
> +    "\t-o output_file: store RMC fingerprint of current board in output_file\n" \
> +    "\t-i input_file: print RMC fingerprint stored in input_file\n\n" \
> +  "-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
> +    "\t-f intput_file : input fingerprint file to be packed in record\n\n" \
> +    "\tNOTE: RMC will create a fingerprint for the board and use it to\n" \
> +    "\tgenerate record if an input fingerprint file is not provided.\n\n" \
> +    "\t-b: files to be packed in record\n\n" \
> +  "-G: generate rmc database file with records specified in record file list\n\n" \
> +  "-B: get a file blob with specified name associated to the board rmc is\n" \
> +  "running on\n" \
> +    "\t-d: database file to be queried\n" \
> +    "\t-o: path and name of output file of a specific command\n\n" \
> +    "Examples (Steps in an order to add board support into rmc):\n\n" \
> +    "1. Generate board fingerprint:\n" \
> +    "\t./rmc -F\n\n” \
Why do we force the rmc in current dir here? rmc can be installed to a system path like other programs.

> +    "2. Generate a rmc record for the board with two file blobs and save it\n" \
> +    "to a specified file:\n" \
> +    "\t./rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
> +    "3. Generate a rmc database file with records from 3 different boards:\n" \
> +    "\t./rmc -D board1_record board2_record board3_record\n\n" \
> +    "4. Query a file blob named audio.conf associated to the board rmc is\n" \
> +    "running on in database my_rmc.db and output to /tmp/new_audio.conf:\n" \
> +    "\t./rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
> 
> 
> #define RMC_OPT_CAP_F   (1 << 0)
> @@ -51,6 +53,7 @@
> #define RMC_OPT_O       (1 << 5)
> #define RMC_OPT_B       (1 << 6)
> #define RMC_OPT_D       (1 << 7)
> +#define RMC_OPT_I       (1 << 8)
> 
> static void usage () {
>     fprintf(stdout, USAGE);
> @@ -78,7 +81,7 @@ static void dump_fingerprint(rmc_fingerprint_t *fp) {
> static int write_fingerprint_file(const char* pathname, rmc_fingerprint_t *fp) {
>     int i;
>     int first = 0;
> -
> +    /* TODO - do we need to open/close file multiple times to write each field */
>     for (i = 0; i < RMC_FINGER_NUM; i++) {
>         if (write_file(pathname, &fp->rmc_fingers[i].type, sizeof(fp->rmc_fingers[i].type), first))
>             return 1;
> @@ -214,7 +217,6 @@ read_fp_done:
> static rmc_file_t *read_policy_file(char *pathname, int type) {
>     rmc_file_t *tmp = NULL;
>     rmc_size_t policy_len = 0;
> -    int ret;
Any reduction to this project is welcome!

it’s just irrelevant for the purposes claimed in commit msg. Please have another patch for other improvements. (Well, I myself could have violated these rules too)

>     char *path_token;
> 
>     if ((tmp = calloc(1, sizeof(rmc_file_t))) == NULL) {
> @@ -226,8 +228,7 @@ static rmc_file_t *read_policy_file(char *pathname, int type) {
>     tmp->next = NULL;
> 
>     if (type == RMC_GENERIC_FILE) {
> -        ret = read_file(pathname, (char **)&tmp->blob, &policy_len);
> -        if (ret) {
> +        if (read_file(pathname, (char **)&tmp->blob, &policy_len)) {
>             fprintf(stderr, "Failed to read file %s\n\n", pathname);
>             free(tmp);
>             return NULL;
> @@ -311,7 +312,7 @@ int main(int argc, char **argv){
>     /* parse options */
>     opterr = 0;
> 
> -    while ((c = getopt(argc, argv, "FRD:B:b:f:o:d:")) != -1)
> +    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
>         switch (c) {
>         case 'F':
>             options |= RMC_OPT_CAP_F;
> @@ -352,6 +353,10 @@ int main(int argc, char **argv){
>             output_path = optarg;
>             options |= RMC_OPT_O;
>             break;
> +        case 'i':
> +            input_fingerprint = optarg;
> +            options |= RMC_OPT_I;
> +            break;
>         case 'f':
>             input_fingerprint = optarg;
>             options |= RMC_OPT_F;
> @@ -388,7 +393,8 @@ int main(int argc, char **argv){
>             break;
>         case '?':
>             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
> -                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd')
> +                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
> +                    || optopt == 'i')
>                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
>             else if (isprint(optopt))
>                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
> @@ -414,6 +420,15 @@ int main(int argc, char **argv){
>         }
>     }
> 
> +    /* sanity check for -i */
> +    if (options & RMC_OPT_I) {
> +        if (!(options & RMC_OPT_CAP_F)) {
> +            fprintf(stderr, "\nWRONG: Option -i cannot be applied without -F\n\n");
> +            usage();
> +            return 1;
> +        }
> +    }
> +
>     /* sanity check for -R */
>     if ((options & RMC_OPT_CAP_R) && (!(options & RMC_OPT_B))) {
>         fprintf(stderr, "\nWRONG: -b is required when -R is present\n\n");
> @@ -563,25 +578,41 @@ int main(int argc, char **argv){
>     }
> 
>     if (options & RMC_OPT_CAP_F) {
> -        /* set a default fingerprint file name if user didn't provide one */
> -        if (!output_path)
> -            output_path = "rmc.fingerprint";
> +        /* print fingerpring file to console*/
> +        if (options & RMC_OPT_I) {
> +            rmc_fingerprint_t fp;
> +            /* read fingerprint file*/
> +            if (input_fingerprint != NULL) {
> +                if (read_fingerprint_from_file(input_fingerprint, &fp, &raw_fp)) {
> +                    fprintf(stderr, "Cannot read fingerprint from %s\n\n",
> +                    input_fingerprint);
> +                    goto main_free;
> +                }
> +                printf("Successfully read fingerprint from %s \n", input_fingerprint);
> +                dump_fingerprint(&fp);
> +            }else {
> +                printf("Fingerprint file not provided! Exiting.\n");
> +            }
> +        } else { /* generate fingerprint file for the current board*/
> +            /* set a default fingerprint file name if user didn't provide one */
> +            if (!output_path)
> +                output_path = "rmc.fingerprint";
> 
> -        if (rmc_get_fingerprint(&fingerprint)) {
> -            fprintf(stderr, "Cannot get board fingerprint\n");
> -            goto main_free;
> -        }
> +            if (rmc_get_fingerprint(&fingerprint)) {
> +                fprintf(stderr, "Cannot get board fingerprint\n");
> +                goto main_free;
> +            }
> 
> -        printf("Got Fingerprint for board:\n\n");
> -        dump_fingerprint(&fingerprint);
> +            printf("Got Fingerprint for board:\n\n");
> +            dump_fingerprint(&fingerprint);
> 
> -        if (write_fingerprint_file(output_path, &fingerprint)) {
> -            fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
> +            if (write_fingerprint_file(output_path, &fingerprint)) {
> +                fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
> +                rmc_free_fingerprint(&fingerprint);
> +                goto main_free;
> +            }
>             rmc_free_fingerprint(&fingerprint);
> -            goto main_free;
>         }
> -
> -        rmc_free_fingerprint(&fingerprint);
>     }
> 
>     ret = 0;
> -- 
> 2.11.0
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] rmc: add database extraction functionality
  2017-02-02 22:37 ` [PATCH 3/3] rmc: add database extraction functionality Todor Minchev
@ 2017-02-06 21:09   ` Jianxun Zhang
  2017-02-06 23:09     ` Todor Minchev
  0 siblings, 1 reply; 11+ messages in thread
From: Jianxun Zhang @ 2017-02-06 21:09 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto

Todor,
Nice change overall. I haven’t run any test and just share multiple (11) inline comments for this patch.

This should be the last one in the series. Please let me know if I missed any other RMC patches for review.

I plan to run rmc internal test once we have an updated patch set. We could need to add a new test case for the dumping feature in the future.

You can refer to the README in rmc project for the pipeline of merging.

Thanks!

> On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> The contents of an existing database file can be extracted in the
> current working directory with the -E option. The top level of the
> directory tree is rmc_db_dump and all files corresponding to
> a given record will be saved in a separate sub-directory. The sub-directory
> name of each record is the signature corresponding to the fingerprint for
> that record.
> 
> Example:
> ./src/rmc -E -d rmc.db
> 
> Successfully extracted rmc.db
> 
> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> ---
> inc/rmc_api.h         |  9 ++++++
> src/lib/api.c         | 85 +++++++++++++++++++++++++++++++++++++++++++++++++--
> src/lib/common/rmcl.c |  3 +-
> src/rmc.c             | 44 +++++++++++++++++++-------
> 4 files changed, 126 insertions(+), 15 deletions(-)
> 
> diff --git a/inc/rmc_api.h b/inc/rmc_api.h
> index a484389..ce26220 100644
> --- a/inc/rmc_api.h
> +++ b/inc/rmc_api.h
> @@ -74,6 +74,15 @@ extern int rmc_query_file_by_fp(rmc_fingerprint_t *fp, char *db_pathname, char *
>  */
> extern int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file);
> 
> +
> +/* extract the contents of a database file and store the files corresponding to
> + * each record in a separate directory. The name of each directory is the signature
> + * of the fingerpring for that record
> + * (in) db_pathname: The path and file name of a RMC database file generated by RMC tool
> + * return: 0 on success, non-zero on failure.
> + */
> +int dump_db(char *db_pathname) ;
> +
Please move this into section 1.3, somewhere after next line. It doesn’t belong to section 1.2 “double-action API”

> /* 1.3 - Helper APIs */
> 
> /* Free allocated data referred in a fingerprint
> diff --git a/src/lib/api.c b/src/lib/api.c
> index 0adb390..aca8d99 100644
> --- a/src/lib/api.c
> +++ b/src/lib/api.c
> @@ -3,6 +3,7 @@
>  * RMC API implementation for Linux user space
>  */
> 
> +#define _GNU_SOURCE
> #include <stdio.h>
> #include <unistd.h>
> #include <errno.h>
> @@ -14,8 +15,11 @@
> #include <rmcl.h>
> #include <rsmp.h>
> 
> -#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
> -#define SYSTAB_LEN 4096         /* assume 4kb is enough...*/
> +#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
> +#define SYSTAB_LEN       4096             /* assume 4kb is enough...*/
> +#define DB_DUMP_DIR      "./rmc_db_dump"  /* directory to store db data dump */
> +
> +extern const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN];
We could have a new small helper API like is_rmcdb(db_blob) in RMCL to hold checker logic at line 357 in this file, so that we can get rid of this line and make the checker reusable. (So far I feel the checker should work in both EFI and Linux contexts.)

We could even update checker API without bothering its callers in the future. Let me know if it makes sense...

> 
> int read_file(const char *pathname, char **data, rmc_size_t* len) {
>     int fd = -1;
> @@ -325,3 +329,80 @@ int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file) {
> 
>     return ret;
> }
> +
> +/*
> + * Dump contents of database file
> + * (in) rmc_db - input database file to extract
rmc_db VS db_pathname. I think we can remove the comment here, it is already in the header file.
> + */
> +int dump_db(char *db_pathname) {
> +    rmc_meta_header_t meta_header;
> +    rmc_db_header_t *db_header = NULL;
> +    rmc_record_header_t record_header;
> +    rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
> +    rmc_uint64_t meta_idx = 0;     /* offset of each meta in a record */
> +    rmc_uint64_t file_idx = 0;     /* offset of file in a meta */
> +    rmc_file_t file;
> +    char *out_dir = NULL, *out_name = NULL;
> +    rmc_size_t db_len = 0;
> +    rmc_uint8_t *rmc_db = NULL;
> +    struct stat s = {0};
> +
> +    if (read_file(db_pathname, (char **)&rmc_db, &db_len)) {
> +        fprintf(stderr, "Failed to read database file\n\n");
> +        return 1;
> +    }
> +
> +    db_header = (rmc_db_header_t *)rmc_db;
> +
> +    /* sanity check of db */
> +    if (strncmp((const char *)db_header->signature,
> +        (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> +        return 1;
> +
> +    /* create the top level directory */
> +    if (stat(DB_DUMP_DIR, &s) == -1) {
> +        if(mkdir(DB_DUMP_DIR, 0755)) {
> +            fprintf(stderr, "Failed to create %s directory\n\n", out_name);
I think we should not go any further when we cannot create the top dir.
> +        }
> +    }
> +
> +    /* query the meta. idx: start of record */
> +    record_idx = sizeof(rmc_db_header_t);
> +    while (record_idx < db_header->length) {
> +        memcpy(&record_header, rmc_db + record_idx,
> +            sizeof(rmc_record_header_t));
> +
> +        /* directory name is fingerprint signature */
> +        asprintf(&out_dir, "%s/%s/", DB_DUMP_DIR, record_header.signature.raw);
Technically, what we get from firmware could contain any kinds of chars. I guess there are some corner cases when chars are not accepted in a dir name.

> +        if (stat(out_dir, &s) == -1) {
> +            if(mkdir(out_dir, 0755)) {
> +                fprintf(stderr, "Failed to create %s directory\n\n", out_name);
out_name -> out_dir

> +            }
> +        }
> +
> +        /* find meta */
> +        for (meta_idx = record_idx + sizeof(rmc_record_header_t);
> +            meta_idx < record_idx + record_header.length;) {
> +            memcpy(&meta_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
> +            file_idx = meta_idx + sizeof(rmc_meta_header_t);
> +            rmc_ssize_t name_len = strlen((char *)&rmc_db[file_idx]) + 1;
> +            file.blob = &rmc_db[file_idx + name_len];
> +            file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
> +                name_len;
> +            file.next = NULL;
> +            file.type = RMC_GENERIC_FILE;
> +            asprintf(&out_name, "%s%s", out_dir, (char *)&rmc_db[file_idx]);
> +            /* write file to dump directory */
> +            if (write_file((const char *)out_name, file.blob, file.blob_len, 0))
> +                return 1;
> +
> +            /* next meta */
> +            meta_idx += meta_header.length;
> +            free(out_name);
> +        }
> +        /* next record */
> +        record_idx += record_header.length;
> +        free(out_dir);
> +    }
> +    return 0;
> +}
> diff --git a/src/lib/common/rmcl.c b/src/lib/common/rmcl.c
> index 67622a0..c996577 100644
> --- a/src/lib/common/rmcl.c
> +++ b/src/lib/common/rmcl.c
> @@ -10,7 +10,7 @@
> #include <rmc_util.h>
> #endif
> 
> -static const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN] = {'R', 'M', 'C', 'D', 'B'};
> +const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN] = {'R', 'M', 'C', 'D', 'B’};
Refer to my first comment. And signature should be internal in rmcl.
> 
> /* compute a finger to signature which is stored in record
>  * (in) fingerprint : of board, usually generated by rmc tool and rsmp
> @@ -242,7 +242,6 @@ int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rm
>                         policy->blob_len = meta_header.length - sizeof(rmc_meta_header_t) - cmd_name_len;
>                         policy->next = NULL;
>                         policy->type = type;
> -
I usually insert a new line before a return in rmc project as a loose rule, assuming this is an intentional change.
>                         return 0;
>                     }
>                 }
> diff --git a/src/rmc.c b/src/rmc.c
> index a051ccf..888cbdb 100644
> --- a/src/rmc.c
> +++ b/src/rmc.c
> @@ -32,6 +32,8 @@
>   "running on\n" \
>     "\t-d: database file to be queried\n" \
>     "\t-o: path and name of output file of a specific command\n\n" \
> +  "-E: Extract database data to current working directory\n” \
I believe user should be able to specify the output dir. DB_DUMP_DIR can serve as the default.

> +    "\t-d: database file to extract\n\n" \
>     "Examples (Steps in an order to add board support into rmc):\n\n" \
>     "1. Generate board fingerprint:\n" \
>     "\t./rmc -F\n\n" \
> @@ -49,11 +51,12 @@
> #define RMC_OPT_CAP_R   (1 << 1)
> #define RMC_OPT_CAP_D   (1 << 2)
> #define RMC_OPT_CAP_B   (1 << 3)
> -#define RMC_OPT_F       (1 << 4)
> -#define RMC_OPT_O       (1 << 5)
> -#define RMC_OPT_B       (1 << 6)
> -#define RMC_OPT_D       (1 << 7)
> -#define RMC_OPT_I       (1 << 8)
> +#define RMC_OPT_CAP_E   (1 << 4)
> +#define RMC_OPT_F       (1 << 5)
> +#define RMC_OPT_O       (1 << 6)
> +#define RMC_OPT_B       (1 << 7)
> +#define RMC_OPT_D       (1 << 8)
> +#define RMC_OPT_I       (1 << 9)
> 
> static void usage () {
>     fprintf(stdout, USAGE);
> @@ -312,7 +315,7 @@ int main(int argc, char **argv){
>     /* parse options */
>     opterr = 0;
> 
> -    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
> +    while ((c = getopt(argc, argv, "FRED:B:b:f:o:i:d:")) != -1)
>         switch (c) {
>         case 'F':
>             options |= RMC_OPT_CAP_F;
> @@ -320,6 +323,9 @@ int main(int argc, char **argv){
>         case 'R':
>             options |= RMC_OPT_CAP_R;
>             break;
> +        case 'E':
> +            options |= RMC_OPT_CAP_E;
> +            break;
>         case 'D':
>             /* we don't know number of arguments for this option at this point,
>              * allocate array with argc which is bigger than needed. But we also
> @@ -393,8 +399,8 @@ int main(int argc, char **argv){
>             break;
>         case '?':
>             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
> -                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
> -                    || optopt == 'i')
> +                    optopt == 'E' ||  optopt == 'b' || optopt == 'f' || \
> +                    optopt == 'o' || optopt == 'd' || optopt == 'i')
>                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
>             else if (isprint(optopt))
>                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
> @@ -436,6 +442,13 @@ int main(int argc, char **argv){
>         return 1;
>     }
> 
> +    /* sanity check for -E */
> +    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_D))) {
> +        fprintf(stderr, "\nERROR: -E requires -d <database file name>\n\n");
> +        usage();
> +        return 1;
> +    }
> +
>     /* sanity check for -B */
>     if ((options & RMC_OPT_CAP_B) && (!(options & RMC_OPT_D) || !(options & RMC_OPT_O))) {
>         fprintf(stderr, "\nWRONG: -B requires -d and -o\n\n");
> @@ -448,7 +461,8 @@ int main(int argc, char **argv){
>         rmc_file_t file;
> 
>         if (!output_path) {
> -            fprintf(stderr, "-B internal error, with -o but no output pathname specified\n\n");
> +            fprintf(stderr, "-B internal error, with -o but no output \
> +                pathname specified\n\n”);
Irrelevant change...
>             goto main_free;
>         }
> 
> @@ -456,14 +470,22 @@ int main(int argc, char **argv){
>             goto main_free;
> 
>         if (write_file(output_path, file.blob, file.blob_len, 0)) {
> -            fprintf(stderr, "-B failed to write file %s to %s\n\n", input_blob_name, output_path);
> +            fprintf(stderr, "-B failed to write file %s to %s\n\n",
> +                input_blob_name, output_path);
Irrelevant change...
>             rmc_free_file(&file);
>             goto main_free;
>         }
> -
>         rmc_free_file(&file);
>     }
> 
> +    /* dump database data */
> +    if (options & RMC_OPT_CAP_E) {
> +        if(dump_db(input_db_path_d))
> +            fprintf(stderr, "\nFailed to extract %s\n\n", input_db_path_d);
> +        else
> +            printf("\nSuccessfully extracted %s\n\n", input_db_path_d);
> +    }
> +
>     /* generate RMC database file */
>     if (options & RMC_OPT_CAP_D) {
>         int record_idx = 0;
> -- 
> 2.11.0
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] Makefile: add verbosity and debug options to Makefile
  2017-02-06 19:06   ` Jianxun Zhang
@ 2017-02-06 22:21     ` Todor Minchev
  0 siblings, 0 replies; 11+ messages in thread
From: Todor Minchev @ 2017-02-06 22:21 UTC (permalink / raw)
  To: Jianxun Zhang; +Cc: meta-intel, yocto

On Mon, 2017-02-06 at 11:06 -0800, Jianxun Zhang wrote:
> Todor,
> Please refer to my 2 inline comments.
> 
> > On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> > 
> > By default Makefile verbosity is disabled (V=0). Verbosity can be enabled by
> > setting the V environment variable to any value not equal to 0 (e.g V=1)
> > 
> > Example:
> > make clean V=1; make V=1
> > 
> > A debug version of the rmc binary can be built by using the debug
> > Makefile target. This will include debug symbols and will disable compiler
> > optimizations when compiling rmc.
> > 
> > Example:
> > 
> > make debug
> > 
> > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > ---
> > Makefile | 31 +++++++++++++++++++++----------
> > 1 file changed, 21 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 9ade775..d85d8e9 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1,5 +1,12 @@
> > # Copyright (C) 2016 Jianxun Zhang <jianxun.zhang@intel.com>
> > 
> > +V ?= 0
> > +ifeq ($(V),0)
> > +  VERBOSITY = @
> > +else
> > +  VERBOSITY =
> > +endif
> > +
> I am thinking maybe we should remove ‘@‘ in rules and use -s option of make (in recipe) when we want to mute the output. This should reach the same effect without bothering new variables.
> 
> We still enable/disable output as a whole anyway.
> 
> Let me know if this proposal works for you.

Sounds good. Removing the @ will make it possible to toggle the
verbosity with 'make -s' without introducing any extra variables. 

> 
> > TOPDIR = $(shell if [ -z "$$PWD" ]; then pwd; else echo "$$PWD"; fi)
> > 
> > RMC_TOOL_SRC := $(wildcard src/*.c)
> > @@ -20,28 +27,32 @@ RMC_INSTALL_HEADER_PATH := $(RMC_INSTALL_PREFIX)/include/rmc/
> > 
> > ALL_OBJS := $(RMC_TOOL_OBJ) $(RMC_LIB_OBJ)
> > 
> > +
> > RMC_CFLAGS := -Wall -I$(TOPDIR)/inc
> > 
> > all: rmc
> > +debug: RMC_CFLAGS += -DDEBUG -g -O0
> > +debug: rmc
> I am not sure if this is necessary because we already have CFLAGS. I think you can reach the same effect without adding a new debug target:
> make CFLAGS='-DDEBUG -g -O0’
> 

Yup, the above will have the same effect as 'make debug'.
I thought that 'make debug' might be a convenient way to build a debug
binary? Do you think it makes sense to keep this extra target for
convenience since it doesn't affect the other usages of make?

> also refer to commit e8b48e6 in rmc project.
> 
> > 
> > $(ALL_OBJS): %.o: %.c
> > -	@$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> > +	$(VERBOSITY)$(CC) -c $(CFLAGS) $(RMC_CFLAGS) $< -o $@
> > 
> > librmc: $(RMC_LIB_OBJ)
> > -	@$(AR) rcs src/lib/$@.a $^
> > +	$(VERBOSITY)$(AR) rcs src/lib/$@.a $^
> > 
> > rmc: $(RMC_TOOL_OBJ) librmc
> > -	@$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) src/lib/librmc.a -o src/$@
> > +	$(VERBOSITY)$(CC) $(CFLAGS) $(RMC_CFLAGS) -Lsrc/lib/ -lrmc $(RMC_TOOL_OBJ) \
> > +  src/lib/librmc.a -o src/$@
> > 
> > clean:
> > -	@rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> > +	$(VERBOSITY)rm -f $(ALL_OBJS) src/rmc src/lib/librmc.a
> > 
> > .PHONY: clean rmc librmc
> > 
> > install:
> > -	@mkdir -p $(RMC_INSTALL_BIN_PATH)
> > -	@install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> > -	@mkdir -p $(RMC_INSTALL_LIB_PATH)
> > -	@install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> > -	@mkdir -p $(RMC_INSTALL_HEADER_PATH)
> > -	@install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> > +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_BIN_PATH)
> > +	$(VERBOSITY)install -m 755 src/rmc $(RMC_INSTALL_BIN_PATH)
> > +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_LIB_PATH)
> > +	$(VERBOSITY)install -m 644 src/lib/librmc.a $(RMC_INSTALL_LIB_PATH)
> > +	$(VERBOSITY)mkdir -p $(RMC_INSTALL_HEADER_PATH)
> > +	$(VERBOSITY)install -m 644 $(RMC_INSTALL_HEADERS) $(RMC_INSTALL_HEADER_PATH)
> > -- 
> > 2.11.0
> > 
> 




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file
  2017-02-06 20:01   ` Jianxun Zhang
@ 2017-02-06 22:28     ` Todor Minchev
  2017-02-06 23:12       ` Jianxun Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Todor Minchev @ 2017-02-06 22:28 UTC (permalink / raw)
  To: Jianxun Zhang; +Cc: meta-intel, yocto

On Mon, 2017-02-06 at 12:01 -0800, Jianxun Zhang wrote:
> Tudor,
> Please refer to my 3 inline comments.
> 
> > On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> > 
> > The contents of an existing fingerprint file can be read and output on
> > the command line with the following options:
> > 
> > ./rmc -F -i input_fingerprint_file
> Suggest we have a new top option for dumping in parallel with -F to keep usages clear and simple for users.

We can use -E to extract both the database and the fingerprint?

rmc -E -d rmc.db
rmc -E -f rmc.fingerprint

> > 
> > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > ---
> > src/rmc.c | 121 +++++++++++++++++++++++++++++++++++++++-----------------------
> > 1 file changed, 76 insertions(+), 45 deletions(-)
> > 
> > diff --git a/src/rmc.c b/src/rmc.c
> > index 062dd36..a051ccf 100644
> > --- a/src/rmc.c
> > +++ b/src/rmc.c
> > @@ -14,33 +14,35 @@
> > #include <rmc_api.h>
> > 
> > #define USAGE "RMC (Runtime Machine configuration) Tool\n" \
> > -    "NOTE: Most of usages require root permission (sudo)\n" \
> > -    "rmc -F [-o output_fingerprint]\n" \
> > +    "NOTE: Most of usages require root permission (sudo)\n\n" \
> > +    "rmc -F [-o output_fingerprint] | -i input_fingerprint\n" \
> >     "rmc -R [-f <fingerprint file>] -b <blob file list> [-o output_record]\n" \
> >     "rmc -D <rmc record file list> [-o output_database]\n" \
> > -	"rmc -B <name of file blob> -d <rmc database file> -o output_file\n" \
> > -	"\n" \
> > -	"-F: generate board rmc fingerprint of board\n" \
> > -	"-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
> > -    "-f: fingerprint file to be packed in record, rmc will create a fingerprint for board and use it internally to\n" \
> > -    "    generate record if -f is missed.\n" \
> > -    "-b: files to be packed in record\n" \
> > -	"-G: generate rmc database file with records specified in record file list\n" \
> > -	"-B: get a flie blob with specified name associated to the board rmc is running on\n" \
> > -	"-d: database file to be queried\n" \
> > -	"-o: path and name of output file of a specific command\n" \
> > -	"\n" \
> > -    "Examples (Steps in an order to add board support into rmc):\n" \
> > -    "generate board fingerprint:\n" \
> > -    "rmc -F\n\n" \
> > -    "generate a rmc record for the board with two file blobs, output to:\n" \
> > -    "a specified file:\n" \
> > -    "rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
> > -    "generate a rmc database file with records from 3 different boards:\n" \
> > -    "rmc -D board1_record board2_record board3_record\n\n" \
> > -    "query a file blob named audio.conf associated to the board rmc is running on in database my_rmc.db and output\n" \
> > -    "to /tmp/new_audio.conf:\n" \
> > -    "rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
> > +    "rmc -B <name of file blob> -d <rmc database file> -o output_file\n\n" \
> > +  "-F: manage fingerprint file\n" \
> > +    "\t-o output_file: store RMC fingerprint of current board in output_file\n" \
> > +    "\t-i input_file: print RMC fingerprint stored in input_file\n\n" \
> > +  "-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
> > +    "\t-f intput_file : input fingerprint file to be packed in record\n\n" \
> > +    "\tNOTE: RMC will create a fingerprint for the board and use it to\n" \
> > +    "\tgenerate record if an input fingerprint file is not provided.\n\n" \
> > +    "\t-b: files to be packed in record\n\n" \
> > +  "-G: generate rmc database file with records specified in record file list\n\n" \
> > +  "-B: get a file blob with specified name associated to the board rmc is\n" \
> > +  "running on\n" \
> > +    "\t-d: database file to be queried\n" \
> > +    "\t-o: path and name of output file of a specific command\n\n" \
> > +    "Examples (Steps in an order to add board support into rmc):\n\n" \
> > +    "1. Generate board fingerprint:\n" \
> > +    "\t./rmc -F\n\n” \
> Why do we force the rmc in current dir here? rmc can be installed to a system path like other programs.

Yes, this can be anywhere.

> 
> > +    "2. Generate a rmc record for the board with two file blobs and save it\n" \
> > +    "to a specified file:\n" \
> > +    "\t./rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
> > +    "3. Generate a rmc database file with records from 3 different boards:\n" \
> > +    "\t./rmc -D board1_record board2_record board3_record\n\n" \
> > +    "4. Query a file blob named audio.conf associated to the board rmc is\n" \
> > +    "running on in database my_rmc.db and output to /tmp/new_audio.conf:\n" \
> > +    "\t./rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
> > 
> > 
> > #define RMC_OPT_CAP_F   (1 << 0)
> > @@ -51,6 +53,7 @@
> > #define RMC_OPT_O       (1 << 5)
> > #define RMC_OPT_B       (1 << 6)
> > #define RMC_OPT_D       (1 << 7)
> > +#define RMC_OPT_I       (1 << 8)
> > 
> > static void usage () {
> >     fprintf(stdout, USAGE);
> > @@ -78,7 +81,7 @@ static void dump_fingerprint(rmc_fingerprint_t *fp) {
> > static int write_fingerprint_file(const char* pathname, rmc_fingerprint_t *fp) {
> >     int i;
> >     int first = 0;
> > -
> > +    /* TODO - do we need to open/close file multiple times to write each field */
> >     for (i = 0; i < RMC_FINGER_NUM; i++) {
> >         if (write_file(pathname, &fp->rmc_fingers[i].type, sizeof(fp->rmc_fingers[i].type), first))
> >             return 1;
> > @@ -214,7 +217,6 @@ read_fp_done:
> > static rmc_file_t *read_policy_file(char *pathname, int type) {
> >     rmc_file_t *tmp = NULL;
> >     rmc_size_t policy_len = 0;
> > -    int ret;
> Any reduction to this project is welcome!
> 
> it’s just irrelevant for the purposes claimed in commit msg. Please have another patch for other improvements. (Well, I myself could have violated these rules too)

OK.. I will split this into its own commit.

> 
> >     char *path_token;
> > 
> >     if ((tmp = calloc(1, sizeof(rmc_file_t))) == NULL) {
> > @@ -226,8 +228,7 @@ static rmc_file_t *read_policy_file(char *pathname, int type) {
> >     tmp->next = NULL;
> > 
> >     if (type == RMC_GENERIC_FILE) {
> > -        ret = read_file(pathname, (char **)&tmp->blob, &policy_len);
> > -        if (ret) {
> > +        if (read_file(pathname, (char **)&tmp->blob, &policy_len)) {
> >             fprintf(stderr, "Failed to read file %s\n\n", pathname);
> >             free(tmp);
> >             return NULL;
> > @@ -311,7 +312,7 @@ int main(int argc, char **argv){
> >     /* parse options */
> >     opterr = 0;
> > 
> > -    while ((c = getopt(argc, argv, "FRD:B:b:f:o:d:")) != -1)
> > +    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
> >         switch (c) {
> >         case 'F':
> >             options |= RMC_OPT_CAP_F;
> > @@ -352,6 +353,10 @@ int main(int argc, char **argv){
> >             output_path = optarg;
> >             options |= RMC_OPT_O;
> >             break;
> > +        case 'i':
> > +            input_fingerprint = optarg;
> > +            options |= RMC_OPT_I;
> > +            break;
> >         case 'f':
> >             input_fingerprint = optarg;
> >             options |= RMC_OPT_F;
> > @@ -388,7 +393,8 @@ int main(int argc, char **argv){
> >             break;
> >         case '?':
> >             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
> > -                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd')
> > +                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
> > +                    || optopt == 'i')
> >                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
> >             else if (isprint(optopt))
> >                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
> > @@ -414,6 +420,15 @@ int main(int argc, char **argv){
> >         }
> >     }
> > 
> > +    /* sanity check for -i */
> > +    if (options & RMC_OPT_I) {
> > +        if (!(options & RMC_OPT_CAP_F)) {
> > +            fprintf(stderr, "\nWRONG: Option -i cannot be applied without -F\n\n");
> > +            usage();
> > +            return 1;
> > +        }
> > +    }
> > +
> >     /* sanity check for -R */
> >     if ((options & RMC_OPT_CAP_R) && (!(options & RMC_OPT_B))) {
> >         fprintf(stderr, "\nWRONG: -b is required when -R is present\n\n");
> > @@ -563,25 +578,41 @@ int main(int argc, char **argv){
> >     }
> > 
> >     if (options & RMC_OPT_CAP_F) {
> > -        /* set a default fingerprint file name if user didn't provide one */
> > -        if (!output_path)
> > -            output_path = "rmc.fingerprint";
> > +        /* print fingerpring file to console*/
> > +        if (options & RMC_OPT_I) {
> > +            rmc_fingerprint_t fp;
> > +            /* read fingerprint file*/
> > +            if (input_fingerprint != NULL) {
> > +                if (read_fingerprint_from_file(input_fingerprint, &fp, &raw_fp)) {
> > +                    fprintf(stderr, "Cannot read fingerprint from %s\n\n",
> > +                    input_fingerprint);
> > +                    goto main_free;
> > +                }
> > +                printf("Successfully read fingerprint from %s \n", input_fingerprint);
> > +                dump_fingerprint(&fp);
> > +            }else {
> > +                printf("Fingerprint file not provided! Exiting.\n");
> > +            }
> > +        } else { /* generate fingerprint file for the current board*/
> > +            /* set a default fingerprint file name if user didn't provide one */
> > +            if (!output_path)
> > +                output_path = "rmc.fingerprint";
> > 
> > -        if (rmc_get_fingerprint(&fingerprint)) {
> > -            fprintf(stderr, "Cannot get board fingerprint\n");
> > -            goto main_free;
> > -        }
> > +            if (rmc_get_fingerprint(&fingerprint)) {
> > +                fprintf(stderr, "Cannot get board fingerprint\n");
> > +                goto main_free;
> > +            }
> > 
> > -        printf("Got Fingerprint for board:\n\n");
> > -        dump_fingerprint(&fingerprint);
> > +            printf("Got Fingerprint for board:\n\n");
> > +            dump_fingerprint(&fingerprint);
> > 
> > -        if (write_fingerprint_file(output_path, &fingerprint)) {
> > -            fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
> > +            if (write_fingerprint_file(output_path, &fingerprint)) {
> > +                fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
> > +                rmc_free_fingerprint(&fingerprint);
> > +                goto main_free;
> > +            }
> >             rmc_free_fingerprint(&fingerprint);
> > -            goto main_free;
> >         }
> > -
> > -        rmc_free_fingerprint(&fingerprint);
> >     }
> > 
> >     ret = 0;
> > -- 
> > 2.11.0
> > 
> 




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] rmc: add database extraction functionality
  2017-02-06 21:09   ` Jianxun Zhang
@ 2017-02-06 23:09     ` Todor Minchev
  0 siblings, 0 replies; 11+ messages in thread
From: Todor Minchev @ 2017-02-06 23:09 UTC (permalink / raw)
  To: Jianxun Zhang; +Cc: meta-intel, yocto

On Mon, 2017-02-06 at 13:09 -0800, Jianxun Zhang wrote:
> Todor,
> Nice change overall. I haven’t run any test and just share multiple (11) inline comments for this patch.

A patchset incorporating these comments is in progress.

> This should be the last one in the series. Please let me know if I missed any other RMC patches for review.
> 
> I plan to run rmc internal test once we have an updated patch set. We could need to add a new test case for the dumping feature in the future.
> 
> You can refer to the README in rmc project for the pipeline of merging.
> 
> Thanks!
> 
> > On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> > 
> > The contents of an existing database file can be extracted in the
> > current working directory with the -E option. The top level of the
> > directory tree is rmc_db_dump and all files corresponding to
> > a given record will be saved in a separate sub-directory. The sub-directory
> > name of each record is the signature corresponding to the fingerprint for
> > that record.
> > 
> > Example:
> > ./src/rmc -E -d rmc.db
> > 
> > Successfully extracted rmc.db
> > 
> > Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
> > ---
> > inc/rmc_api.h         |  9 ++++++
> > src/lib/api.c         | 85 +++++++++++++++++++++++++++++++++++++++++++++++++--
> > src/lib/common/rmcl.c |  3 +-
> > src/rmc.c             | 44 +++++++++++++++++++-------
> > 4 files changed, 126 insertions(+), 15 deletions(-)
> > 
> > diff --git a/inc/rmc_api.h b/inc/rmc_api.h
> > index a484389..ce26220 100644
> > --- a/inc/rmc_api.h
> > +++ b/inc/rmc_api.h
> > @@ -74,6 +74,15 @@ extern int rmc_query_file_by_fp(rmc_fingerprint_t *fp, char *db_pathname, char *
> >  */
> > extern int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file);
> > 
> > +
> > +/* extract the contents of a database file and store the files corresponding to
> > + * each record in a separate directory. The name of each directory is the signature
> > + * of the fingerpring for that record
> > + * (in) db_pathname: The path and file name of a RMC database file generated by RMC tool
> > + * return: 0 on success, non-zero on failure.
> > + */
> > +int dump_db(char *db_pathname) ;
> > +
> Please move this into section 1.3, somewhere after next line. It doesn’t belong to section 1.2 “double-action API”

Will do.

> 
> > /* 1.3 - Helper APIs */
> > 
> > /* Free allocated data referred in a fingerprint
> > diff --git a/src/lib/api.c b/src/lib/api.c
> > index 0adb390..aca8d99 100644
> > --- a/src/lib/api.c
> > +++ b/src/lib/api.c
> > @@ -3,6 +3,7 @@
> >  * RMC API implementation for Linux user space
> >  */
> > 
> > +#define _GNU_SOURCE
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <errno.h>
> > @@ -14,8 +15,11 @@
> > #include <rmcl.h>
> > #include <rsmp.h>
> > 
> > -#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
> > -#define SYSTAB_LEN 4096         /* assume 4kb is enough...*/
> > +#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
> > +#define SYSTAB_LEN       4096             /* assume 4kb is enough...*/
> > +#define DB_DUMP_DIR      "./rmc_db_dump"  /* directory to store db data dump */
> > +
> > +extern const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN];
> We could have a new small helper API like is_rmcdb(db_blob) in RMCL to hold checker logic at line 357 in this file, so that we can get rid of this line and make the checker reusable. (So far I feel the checker should work in both EFI and Linux contexts.)
> 
> We could even update checker API without bothering its callers in the future. Let me know if it makes sense...

Makes sense

> > 
> > int read_file(const char *pathname, char **data, rmc_size_t* len) {
> >     int fd = -1;
> > @@ -325,3 +329,80 @@ int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t *file) {
> > 
> >     return ret;
> > }
> > +
> > +/*
> > + * Dump contents of database file
> > + * (in) rmc_db - input database file to extract
> rmc_db VS db_pathname. I think we can remove the comment here, it is already in the header file.
> > + */

OK

> > +int dump_db(char *db_pathname) {
> > +    rmc_meta_header_t meta_header;
> > +    rmc_db_header_t *db_header = NULL;
> > +    rmc_record_header_t record_header;
> > +    rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
> > +    rmc_uint64_t meta_idx = 0;     /* offset of each meta in a record */
> > +    rmc_uint64_t file_idx = 0;     /* offset of file in a meta */
> > +    rmc_file_t file;
> > +    char *out_dir = NULL, *out_name = NULL;
> > +    rmc_size_t db_len = 0;
> > +    rmc_uint8_t *rmc_db = NULL;
> > +    struct stat s = {0};
> > +
> > +    if (read_file(db_pathname, (char **)&rmc_db, &db_len)) {
> > +        fprintf(stderr, "Failed to read database file\n\n");
> > +        return 1;
> > +    }
> > +
> > +    db_header = (rmc_db_header_t *)rmc_db;
> > +
> > +    /* sanity check of db */
> > +    if (strncmp((const char *)db_header->signature,
> > +        (const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> > +        return 1;
> > +
> > +    /* create the top level directory */
> > +    if (stat(DB_DUMP_DIR, &s) == -1) {
> > +        if(mkdir(DB_DUMP_DIR, 0755)) {
> > +            fprintf(stderr, "Failed to create %s directory\n\n", out_name);
> I think we should not go any further when we cannot create the top dir.

Makes sense

> > +        }
> > +    }
> > +
> > +    /* query the meta. idx: start of record */
> > +    record_idx = sizeof(rmc_db_header_t);
> > +    while (record_idx < db_header->length) {
> > +        memcpy(&record_header, rmc_db + record_idx,
> > +            sizeof(rmc_record_header_t));
> > +
> > +        /* directory name is fingerprint signature */
> > +        asprintf(&out_dir, "%s/%s/", DB_DUMP_DIR, record_header.signature.raw);
> Technically, what we get from firmware could contain any kinds of chars. I guess there are some corner cases when chars are not accepted in a dir name.

We can have the blobs associated with each fingerprint dumped in a
directory with a predefined name and then store the fingerprint
signature in a file in that directory. The user can then figure out what
blobs correspond to what fingerprint. I am also going to get the
signature dumped with the fingerprint dumper e.g


Signature: xxxxxxxxxxxxxxxxxxxxxxx
Finger 0 type   : 0x01
Finger 0 offset : 0x05
Finger 0 name:  : product_name
Finger 0 value  : Super Server

*snip*

> > +        if (stat(out_dir, &s) == -1) {
> > +            if(mkdir(out_dir, 0755)) {
> > +                fprintf(stderr, "Failed to create %s directory\n\n", out_name);
> out_name -> out_dir
> 
> > +            }
> > +        }
> > +
> > +        /* find meta */
> > +        for (meta_idx = record_idx + sizeof(rmc_record_header_t);
> > +            meta_idx < record_idx + record_header.length;) {
> > +            memcpy(&meta_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
> > +            file_idx = meta_idx + sizeof(rmc_meta_header_t);
> > +            rmc_ssize_t name_len = strlen((char *)&rmc_db[file_idx]) + 1;
> > +            file.blob = &rmc_db[file_idx + name_len];
> > +            file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
> > +                name_len;
> > +            file.next = NULL;
> > +            file.type = RMC_GENERIC_FILE;
> > +            asprintf(&out_name, "%s%s", out_dir, (char *)&rmc_db[file_idx]);
> > +            /* write file to dump directory */
> > +            if (write_file((const char *)out_name, file.blob, file.blob_len, 0))
> > +                return 1;
> > +
> > +            /* next meta */
> > +            meta_idx += meta_header.length;
> > +            free(out_name);
> > +        }
> > +        /* next record */
> > +        record_idx += record_header.length;
> > +        free(out_dir);
> > +    }
> > +    return 0;
> > +}
> > diff --git a/src/lib/common/rmcl.c b/src/lib/common/rmcl.c
> > index 67622a0..c996577 100644
> > --- a/src/lib/common/rmcl.c
> > +++ b/src/lib/common/rmcl.c
> > @@ -10,7 +10,7 @@
> > #include <rmc_util.h>
> > #endif
> > 
> > -static const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN] = {'R', 'M', 'C', 'D', 'B'};
> > +const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN] = {'R', 'M', 'C', 'D', 'B’};
> Refer to my first comment. And signature should be internal in rmcl.

OK.. We'll do the database validation with is_rmcdb function.

> > 
> > /* compute a finger to signature which is stored in record
> >  * (in) fingerprint : of board, usually generated by rmc tool and rsmp
> > @@ -242,7 +242,6 @@ int query_policy_from_db(rmc_fingerprint_t *fingerprint, rmc_uint8_t *rmc_db, rm
> >                         policy->blob_len = meta_header.length - sizeof(rmc_meta_header_t) - cmd_name_len;
> >                         policy->next = NULL;
> >                         policy->type = type;
> > -
> I usually insert a new line before a return in rmc project as a loose rule, assuming this is an intentional change.

OK

> >                         return 0;
> >                     }
> >                 }
> > diff --git a/src/rmc.c b/src/rmc.c
> > index a051ccf..888cbdb 100644
> > --- a/src/rmc.c
> > +++ b/src/rmc.c
> > @@ -32,6 +32,8 @@
> >   "running on\n" \
> >     "\t-d: database file to be queried\n" \
> >     "\t-o: path and name of output file of a specific command\n\n" \
> > +  "-E: Extract database data to current working directory\n” \
> I believe user should be able to specify the output dir. DB_DUMP_DIR can serve as the default.

Alright, it will be 'rmc -E -d rmc.db -o out_dir'..

> > +    "\t-d: database file to extract\n\n" \
> >     "Examples (Steps in an order to add board support into rmc):\n\n" \
> >     "1. Generate board fingerprint:\n" \
> >     "\t./rmc -F\n\n" \
> > @@ -49,11 +51,12 @@
> > #define RMC_OPT_CAP_R   (1 << 1)
> > #define RMC_OPT_CAP_D   (1 << 2)
> > #define RMC_OPT_CAP_B   (1 << 3)
> > -#define RMC_OPT_F       (1 << 4)
> > -#define RMC_OPT_O       (1 << 5)
> > -#define RMC_OPT_B       (1 << 6)
> > -#define RMC_OPT_D       (1 << 7)
> > -#define RMC_OPT_I       (1 << 8)
> > +#define RMC_OPT_CAP_E   (1 << 4)
> > +#define RMC_OPT_F       (1 << 5)
> > +#define RMC_OPT_O       (1 << 6)
> > +#define RMC_OPT_B       (1 << 7)
> > +#define RMC_OPT_D       (1 << 8)
> > +#define RMC_OPT_I       (1 << 9)
> > 
> > static void usage () {
> >     fprintf(stdout, USAGE);
> > @@ -312,7 +315,7 @@ int main(int argc, char **argv){
> >     /* parse options */
> >     opterr = 0;
> > 
> > -    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
> > +    while ((c = getopt(argc, argv, "FRED:B:b:f:o:i:d:")) != -1)
> >         switch (c) {
> >         case 'F':
> >             options |= RMC_OPT_CAP_F;
> > @@ -320,6 +323,9 @@ int main(int argc, char **argv){
> >         case 'R':
> >             options |= RMC_OPT_CAP_R;
> >             break;
> > +        case 'E':
> > +            options |= RMC_OPT_CAP_E;
> > +            break;
> >         case 'D':
> >             /* we don't know number of arguments for this option at this point,
> >              * allocate array with argc which is bigger than needed. But we also
> > @@ -393,8 +399,8 @@ int main(int argc, char **argv){
> >             break;
> >         case '?':
> >             if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
> > -                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
> > -                    || optopt == 'i')
> > +                    optopt == 'E' ||  optopt == 'b' || optopt == 'f' || \
> > +                    optopt == 'o' || optopt == 'd' || optopt == 'i')
> >                 fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
> >             else if (isprint(optopt))
> >                 fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
> > @@ -436,6 +442,13 @@ int main(int argc, char **argv){
> >         return 1;
> >     }
> > 
> > +    /* sanity check for -E */
> > +    if ((options & RMC_OPT_CAP_E) && (!(options & RMC_OPT_D))) {
> > +        fprintf(stderr, "\nERROR: -E requires -d <database file name>\n\n");
> > +        usage();
> > +        return 1;
> > +    }
> > +
> >     /* sanity check for -B */
> >     if ((options & RMC_OPT_CAP_B) && (!(options & RMC_OPT_D) || !(options & RMC_OPT_O))) {
> >         fprintf(stderr, "\nWRONG: -B requires -d and -o\n\n");
> > @@ -448,7 +461,8 @@ int main(int argc, char **argv){
> >         rmc_file_t file;
> > 
> >         if (!output_path) {
> > -            fprintf(stderr, "-B internal error, with -o but no output pathname specified\n\n");
> > +            fprintf(stderr, "-B internal error, with -o but no output \
> > +                pathname specified\n\n”);
> Irrelevant change...

I was trying to stick to the 80 columns coding style, but since most of
the lines are already more than 80 columns this probably makes no
sense. 

> >             goto main_free;
> >         }
> > 
> > @@ -456,14 +470,22 @@ int main(int argc, char **argv){
> >             goto main_free;
> > 
> >         if (write_file(output_path, file.blob, file.blob_len, 0)) {
> > -            fprintf(stderr, "-B failed to write file %s to %s\n\n", input_blob_name, output_path);
> > +            fprintf(stderr, "-B failed to write file %s to %s\n\n",
> > +                input_blob_name, output_path);
> Irrelevant change...
Same as above.

> >             rmc_free_file(&file);
> >             goto main_free;
> >         }
> > -
> >         rmc_free_file(&file);
> >     }
> > 
> > +    /* dump database data */
> > +    if (options & RMC_OPT_CAP_E) {
> > +        if(dump_db(input_db_path_d))
> > +            fprintf(stderr, "\nFailed to extract %s\n\n", input_db_path_d);
> > +        else
> > +            printf("\nSuccessfully extracted %s\n\n", input_db_path_d);
> > +    }
> > +
> >     /* generate RMC database file */
> >     if (options & RMC_OPT_CAP_D) {
> >         int record_idx = 0;
> > -- 
> > 2.11.0
> > 
> 




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file
  2017-02-06 22:28     ` Todor Minchev
@ 2017-02-06 23:12       ` Jianxun Zhang
  0 siblings, 0 replies; 11+ messages in thread
From: Jianxun Zhang @ 2017-02-06 23:12 UTC (permalink / raw)
  To: Todor Minchev; +Cc: meta-intel, yocto


> On Feb 6, 2017, at 2:28 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
> 
> On Mon, 2017-02-06 at 12:01 -0800, Jianxun Zhang wrote:
>> Tudor,
>> Please refer to my 3 inline comments.
>> 
>>> On Feb 2, 2017, at 2:37 PM, Todor Minchev <todor.minchev@linux.intel.com> wrote:
>>> 
>>> The contents of an existing fingerprint file can be read and output on
>>> the command line with the following options:
>>> 
>>> ./rmc -F -i input_fingerprint_file
>> Suggest we have a new top option for dumping in parallel with -F to keep usages clear and simple for users.
> 
> We can use -E to extract both the database and the fingerprint?
> 
> rmc -E -d rmc.db
> rmc -E -f rmc.fingerprint
good idea.
> 
>>> 
>>> Signed-off-by: Todor Minchev <todor.minchev@linux.intel.com>
>>> ---
>>> src/rmc.c | 121 +++++++++++++++++++++++++++++++++++++++-----------------------
>>> 1 file changed, 76 insertions(+), 45 deletions(-)
>>> 
>>> diff --git a/src/rmc.c b/src/rmc.c
>>> index 062dd36..a051ccf 100644
>>> --- a/src/rmc.c
>>> +++ b/src/rmc.c
>>> @@ -14,33 +14,35 @@
>>> #include <rmc_api.h>
>>> 
>>> #define USAGE "RMC (Runtime Machine configuration) Tool\n" \
>>> -    "NOTE: Most of usages require root permission (sudo)\n" \
>>> -    "rmc -F [-o output_fingerprint]\n" \
>>> +    "NOTE: Most of usages require root permission (sudo)\n\n" \
>>> +    "rmc -F [-o output_fingerprint] | -i input_fingerprint\n" \
>>>    "rmc -R [-f <fingerprint file>] -b <blob file list> [-o output_record]\n" \
>>>    "rmc -D <rmc record file list> [-o output_database]\n" \
>>> -	"rmc -B <name of file blob> -d <rmc database file> -o output_file\n" \
>>> -	"\n" \
>>> -	"-F: generate board rmc fingerprint of board\n" \
>>> -	"-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
>>> -    "-f: fingerprint file to be packed in record, rmc will create a fingerprint for board and use it internally to\n" \
>>> -    "    generate record if -f is missed.\n" \
>>> -    "-b: files to be packed in record\n" \
>>> -	"-G: generate rmc database file with records specified in record file list\n" \
>>> -	"-B: get a flie blob with specified name associated to the board rmc is running on\n" \
>>> -	"-d: database file to be queried\n" \
>>> -	"-o: path and name of output file of a specific command\n" \
>>> -	"\n" \
>>> -    "Examples (Steps in an order to add board support into rmc):\n" \
>>> -    "generate board fingerprint:\n" \
>>> -    "rmc -F\n\n" \
>>> -    "generate a rmc record for the board with two file blobs, output to:\n" \
>>> -    "a specified file:\n" \
>>> -    "rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
>>> -    "generate a rmc database file with records from 3 different boards:\n" \
>>> -    "rmc -D board1_record board2_record board3_record\n\n" \
>>> -    "query a file blob named audio.conf associated to the board rmc is running on in database my_rmc.db and output\n" \
>>> -    "to /tmp/new_audio.conf:\n" \
>>> -    "rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
>>> +    "rmc -B <name of file blob> -d <rmc database file> -o output_file\n\n" \
>>> +  "-F: manage fingerprint file\n" \
>>> +    "\t-o output_file: store RMC fingerprint of current board in output_file\n" \
>>> +    "\t-i input_file: print RMC fingerprint stored in input_file\n\n" \
>>> +  "-R: generate board rmc record of board with its fingerprint and file blobs.\n" \
>>> +    "\t-f intput_file : input fingerprint file to be packed in record\n\n" \
>>> +    "\tNOTE: RMC will create a fingerprint for the board and use it to\n" \
>>> +    "\tgenerate record if an input fingerprint file is not provided.\n\n" \
>>> +    "\t-b: files to be packed in record\n\n" \
>>> +  "-G: generate rmc database file with records specified in record file list\n\n" \
>>> +  "-B: get a file blob with specified name associated to the board rmc is\n" \
>>> +  "running on\n" \
>>> +    "\t-d: database file to be queried\n" \
>>> +    "\t-o: path and name of output file of a specific command\n\n" \
>>> +    "Examples (Steps in an order to add board support into rmc):\n\n" \
>>> +    "1. Generate board fingerprint:\n" \
>>> +    "\t./rmc -F\n\n” \
>> Why do we force the rmc in current dir here? rmc can be installed to a system path like other programs.
> 
> Yes, this can be anywhere.
> 
>> 
>>> +    "2. Generate a rmc record for the board with two file blobs and save it\n" \
>>> +    "to a specified file:\n" \
>>> +    "\t./rmc -R -f fingerprint -b file_1 file_2 -o my_board.record\n\n" \
>>> +    "3. Generate a rmc database file with records from 3 different boards:\n" \
>>> +    "\t./rmc -D board1_record board2_record board3_record\n\n" \
>>> +    "4. Query a file blob named audio.conf associated to the board rmc is\n" \
>>> +    "running on in database my_rmc.db and output to /tmp/new_audio.conf:\n" \
>>> +    "\t./rmc -B audio.conf -d my_rmc.db -o /tmp/new_audio.conf\n\n"
>>> 
>>> 
>>> #define RMC_OPT_CAP_F   (1 << 0)
>>> @@ -51,6 +53,7 @@
>>> #define RMC_OPT_O       (1 << 5)
>>> #define RMC_OPT_B       (1 << 6)
>>> #define RMC_OPT_D       (1 << 7)
>>> +#define RMC_OPT_I       (1 << 8)
>>> 
>>> static void usage () {
>>>    fprintf(stdout, USAGE);
>>> @@ -78,7 +81,7 @@ static void dump_fingerprint(rmc_fingerprint_t *fp) {
>>> static int write_fingerprint_file(const char* pathname, rmc_fingerprint_t *fp) {
>>>    int i;
>>>    int first = 0;
>>> -
>>> +    /* TODO - do we need to open/close file multiple times to write each field */
>>>    for (i = 0; i < RMC_FINGER_NUM; i++) {
>>>        if (write_file(pathname, &fp->rmc_fingers[i].type, sizeof(fp->rmc_fingers[i].type), first))
>>>            return 1;
>>> @@ -214,7 +217,6 @@ read_fp_done:
>>> static rmc_file_t *read_policy_file(char *pathname, int type) {
>>>    rmc_file_t *tmp = NULL;
>>>    rmc_size_t policy_len = 0;
>>> -    int ret;
>> Any reduction to this project is welcome!
>> 
>> it’s just irrelevant for the purposes claimed in commit msg. Please have another patch for other improvements. (Well, I myself could have violated these rules too)
> 
> OK.. I will split this into its own commit.
> 
>> 
>>>    char *path_token;
>>> 
>>>    if ((tmp = calloc(1, sizeof(rmc_file_t))) == NULL) {
>>> @@ -226,8 +228,7 @@ static rmc_file_t *read_policy_file(char *pathname, int type) {
>>>    tmp->next = NULL;
>>> 
>>>    if (type == RMC_GENERIC_FILE) {
>>> -        ret = read_file(pathname, (char **)&tmp->blob, &policy_len);
>>> -        if (ret) {
>>> +        if (read_file(pathname, (char **)&tmp->blob, &policy_len)) {
>>>            fprintf(stderr, "Failed to read file %s\n\n", pathname);
>>>            free(tmp);
>>>            return NULL;
>>> @@ -311,7 +312,7 @@ int main(int argc, char **argv){
>>>    /* parse options */
>>>    opterr = 0;
>>> 
>>> -    while ((c = getopt(argc, argv, "FRD:B:b:f:o:d:")) != -1)
>>> +    while ((c = getopt(argc, argv, "FRD:B:b:f:o:i:d:")) != -1)
>>>        switch (c) {
>>>        case 'F':
>>>            options |= RMC_OPT_CAP_F;
>>> @@ -352,6 +353,10 @@ int main(int argc, char **argv){
>>>            output_path = optarg;
>>>            options |= RMC_OPT_O;
>>>            break;
>>> +        case 'i':
>>> +            input_fingerprint = optarg;
>>> +            options |= RMC_OPT_I;
>>> +            break;
>>>        case 'f':
>>>            input_fingerprint = optarg;
>>>            options |= RMC_OPT_F;
>>> @@ -388,7 +393,8 @@ int main(int argc, char **argv){
>>>            break;
>>>        case '?':
>>>            if (optopt == 'F' || optopt == 'R' || optopt == 'D' || optopt == 'B' || \
>>> -                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd')
>>> +                    optopt == 'b' || optopt == 'f' || optopt == 'o' || optopt == 'd'  \
>>> +                    || optopt == 'i')
>>>                fprintf(stderr, "\nWRONG USAGE: -%c\n\n", optopt);
>>>            else if (isprint(optopt))
>>>                fprintf(stderr, "Unknown option `-%c'.\n\n", optopt);
>>> @@ -414,6 +420,15 @@ int main(int argc, char **argv){
>>>        }
>>>    }
>>> 
>>> +    /* sanity check for -i */
>>> +    if (options & RMC_OPT_I) {
>>> +        if (!(options & RMC_OPT_CAP_F)) {
>>> +            fprintf(stderr, "\nWRONG: Option -i cannot be applied without -F\n\n");
>>> +            usage();
>>> +            return 1;
>>> +        }
>>> +    }
>>> +
>>>    /* sanity check for -R */
>>>    if ((options & RMC_OPT_CAP_R) && (!(options & RMC_OPT_B))) {
>>>        fprintf(stderr, "\nWRONG: -b is required when -R is present\n\n");
>>> @@ -563,25 +578,41 @@ int main(int argc, char **argv){
>>>    }
>>> 
>>>    if (options & RMC_OPT_CAP_F) {
>>> -        /* set a default fingerprint file name if user didn't provide one */
>>> -        if (!output_path)
>>> -            output_path = "rmc.fingerprint";
>>> +        /* print fingerpring file to console*/
>>> +        if (options & RMC_OPT_I) {
>>> +            rmc_fingerprint_t fp;
>>> +            /* read fingerprint file*/
>>> +            if (input_fingerprint != NULL) {
>>> +                if (read_fingerprint_from_file(input_fingerprint, &fp, &raw_fp)) {
>>> +                    fprintf(stderr, "Cannot read fingerprint from %s\n\n",
>>> +                    input_fingerprint);
>>> +                    goto main_free;
>>> +                }
>>> +                printf("Successfully read fingerprint from %s \n", input_fingerprint);
>>> +                dump_fingerprint(&fp);
>>> +            }else {
>>> +                printf("Fingerprint file not provided! Exiting.\n");
>>> +            }
>>> +        } else { /* generate fingerprint file for the current board*/
>>> +            /* set a default fingerprint file name if user didn't provide one */
>>> +            if (!output_path)
>>> +                output_path = "rmc.fingerprint";
>>> 
>>> -        if (rmc_get_fingerprint(&fingerprint)) {
>>> -            fprintf(stderr, "Cannot get board fingerprint\n");
>>> -            goto main_free;
>>> -        }
>>> +            if (rmc_get_fingerprint(&fingerprint)) {
>>> +                fprintf(stderr, "Cannot get board fingerprint\n");
>>> +                goto main_free;
>>> +            }
>>> 
>>> -        printf("Got Fingerprint for board:\n\n");
>>> -        dump_fingerprint(&fingerprint);
>>> +            printf("Got Fingerprint for board:\n\n");
>>> +            dump_fingerprint(&fingerprint);
>>> 
>>> -        if (write_fingerprint_file(output_path, &fingerprint)) {
>>> -            fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
>>> +            if (write_fingerprint_file(output_path, &fingerprint)) {
>>> +                fprintf(stderr, "Cannot write board fingerprint to %s\n", output_path);
>>> +                rmc_free_fingerprint(&fingerprint);
>>> +                goto main_free;
>>> +            }
>>>            rmc_free_fingerprint(&fingerprint);
>>> -            goto main_free;
>>>        }
>>> -
>>> -        rmc_free_fingerprint(&fingerprint);
>>>    }
>>> 
>>>    ret = 0;
>>> -- 
>>> 2.11.0
>>> 
>> 
> 
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-02-06 23:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-02 22:37 [PATCH 0/3] [meta-intel][rmc] Add fingerprint quering and database extraction functionality to RMC Todor Minchev
2017-02-02 22:37 ` [PATCH 1/3] Makefile: add verbosity and debug options to Makefile Todor Minchev
2017-02-06 19:06   ` Jianxun Zhang
2017-02-06 22:21     ` Todor Minchev
2017-02-02 22:37 ` [PATCH 2/3] rmc: Enable reading the contents of an existing fingerprint file Todor Minchev
2017-02-06 20:01   ` Jianxun Zhang
2017-02-06 22:28     ` Todor Minchev
2017-02-06 23:12       ` Jianxun Zhang
2017-02-02 22:37 ` [PATCH 3/3] rmc: add database extraction functionality Todor Minchev
2017-02-06 21:09   ` Jianxun Zhang
2017-02-06 23:09     ` Todor Minchev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.