Merge pull request #20 from asdf-format/issue-16

Resolves #16: Improve I/O abstraction

Author: embray

.clang-format

@@ -22,15 +22,15 @@ AllowShortLoopsOnASingleLine: false
 ColumnLimit: 100
 MaxEmptyLinesToKeep: 2
 
-PenaltyReturnTypeOnItsOwnLine: 100
+PenaltyReturnTypeOnItsOwnLine: 10000
 PenaltyBreakBeforeFirstCallParameter: 100
 PenaltyBreakString: 1000
 
 BinPackArguments: false
 BinPackParameters: false
 
 BreakBeforeBraces: Attach
-AlignAfterOpenBracket: DontAlign
+AlignAfterOpenBracket: AlwaysBreak
 AlignEscapedNewlines: DontAlign
 AlwaysBreakAfterDefinitionReturnType: None
 AlwaysBreakBeforeMultilineStrings: false

.clang-tidy

@@ -18,7 +18,7 @@ CheckOptions:
   - key: readability-identifier-naming.VariableCase
     value: lower_case
   - key: readability-identifier-naming.GlobalConstantCase
-    value: UPPER_CASE
+    value: lower_case
   - key: readability-identifier-naming.MacroDefinitionCase
     value: UPPER_CASE
   - key: readability-identifier-naming.FunctionCase

.github/workflows/build.yaml

@@ -20,6 +20,10 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest]
+        cc: [gcc, clang]
+        exclude:
+          - os: macos-latest
+            cc: gcc
 
     steps:
       - name: Checkout code
@@ -38,6 +42,10 @@ jobs:
             build-essential \
             libfyaml-dev
 
+          if [ "${{ matrix.cc }}" = "clang" ]; then
+            sudo apt-get install -y clang
+          fi
+
       - name: Install dependencies (macOS)
         if: runner.os == 'macOS'
         run: |
@@ -53,7 +61,7 @@ jobs:
         run: ./autogen.sh
 
       - name: Configure
-        run: ./configure
+        run: ./configure CC=${{ matrix.cc }}
 
       - name: Build
         run: make ${{ github.event.inputs.make_flags }}

Makefile.am

@@ -2,8 +2,8 @@ AM_CFLAGS = -fvisibility=hidden
 AM_CPPFLAGS = -I$(top_srcdir)/src # so that tests also find header files
 ACLOCAL_AMFLAGS = -Im4
 
-src_files = src/block.c src/event.c src/info.c src/parse.c src/parse_util.c src/yaml.c
-src_headers = src/block.h src/event.h src/info.h src/parse.h src/parse_util.h src/yaml.h
+src_files = src/block.c src/event.c src/info.c src/parse.c src/parse_util.c src/stream.c src/yaml.c
+src_headers = src/block.h src/event.h src/info.h src/parse.h src/parse_util.h src/stream.h src/yaml.h
 
 noinst_HEADERS = $(src_headers)
 

configure.ac

@@ -102,4 +102,14 @@ AS_IF([test "x$asdf_build_tool" = "xyes"], [
 
 AC_CONFIG_HEADERS([config.h]) # use config.h instead of passing -D in the command line
 AC_CONFIG_FILES([Makefile tests/Makefile])
+
 AC_OUTPUT
+
+# Report
+CC_VERSION=`$CC --version | head -n 1`
+FYAML_VERSION=`$PKG_CONFIG --modversion libfyaml`
+printf "\n==================== Configuration Summary ====================\n"
+printf "  %18s %s\n" "$PACKAGE_NAME version:" "$VERSION"
+printf "  %18s %s\n" "compiler version:" "$CC_VERSION"
+printf "  %18s %s\n" "libfyaml version:" "$FYAML_VERSION"
+printf "===============================================================\n\n"

src/block.c

@@ -5,4 +5,4 @@
  */
 
 
-const unsigned char ASDF_BLOCK_MAGIC[] = {'\xd3', 'B', 'L', 'K', '\x00'};
+const unsigned char asdf_block_magic[] = {'\xd3', 'B', 'L', 'K'};

src/block.h

@@ -8,7 +8,9 @@
 #include "config.h"
 #endif
 
+#include <stdbool.h>
 #include <stdint.h>
+#include <string.h>
 #include <sys/types.h>
 
 
@@ -22,12 +24,12 @@
 #define ASDF_BLOCK_FLAGS_OFFSET 0
 #define ASDF_BLOCK_COMPRESSION_OFFSET 4
 #define ASDF_BLOCK_ALLOCATED_SIZE_OFFSET 8
-#define ASDF_BLOCK_DATA_SIZE_OFFSET 16
-#define ASDF_BLOCK_USED_SIZE_OFFSET 24
+#define ASDF_BLOCK_USED_SIZE_OFFSET 16
+#define ASDF_BLOCK_DATA_SIZE_OFFSET 24
 #define ASDF_BLOCK_CHECKSUM_OFFSET 32
 
 
-extern const unsigned char ASDF_BLOCK_MAGIC[];
+extern const unsigned char asdf_block_magic[];
 
 
 typedef struct asdf_block_header {
@@ -65,3 +67,14 @@ typedef struct asdf_block_info {
     off_t header_pos;
     off_t data_pos;
 } asdf_block_info_t;
+
+
+/**
+ * Returns `true` if the given buffer begins with the ASDF block magic
+ */
+static inline bool is_block_magic(const char *buf, size_t len) {
+    if (len < ASDF_BLOCK_MAGIC_SIZE)
+        return false;
+
+    return memcmp(buf, asdf_block_magic, (size_t)ASDF_BLOCK_MAGIC_SIZE) == 0;
+}

src/event.c

@@ -3,6 +3,7 @@
  */
 #include <assert.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -59,7 +60,7 @@ int asdf_event_iterate(asdf_parser_t *parser, asdf_event_t *event) {
 
 const char *asdf_event_type_name(asdf_event_type_t event_type) {
     if (event_type >= 0 && event_type < ASDF_EVENT_TYPE_COUNT)
-        return ASDF_EVENT_TYPE_NAMES[event_type];
+        return asdf_event_type_names[event_type];
 
     return "ASDF_UNKNOWN_EVENT";
 }
@@ -107,28 +108,51 @@ void asdf_event_print(const asdf_event_t *event, FILE *file, bool verbose) {
     }
 
     case ASDF_TREE_START_EVENT:
-        fprintf(file, "  Tree start position: %zu (0x%zx)\n", event->payload.tree->start,
-                event->payload.tree->start);
+        fprintf(
+            file,
+            "  Tree start position: %zu (0x%zx)\n",
+            event->payload.tree->start,
+            event->payload.tree->start);
         break;
 
     case ASDF_TREE_END_EVENT:
-        fprintf(file, "  Tree end position: %zu (0x%zx)\n", event->payload.tree->end,
-                event->payload.tree->end);
+        fprintf(
+            file,
+            "  Tree end position: %zu (0x%zx)\n",
+            event->payload.tree->end,
+            event->payload.tree->end);
+        if (event->payload.tree->buf) {
+            size_t tree_size = event->payload.tree->end - event->payload.tree->start - 1;
+            fprintf(
+                file,
+                "%.*s\n",
+                (int)(tree_size > INT_MAX ? INT_MAX : tree_size),
+                event->payload.tree->buf);
+        }
         break;
 
     case ASDF_BLOCK_EVENT: {
         const asdf_block_info_t *block = event->payload.block;
         const asdf_block_header_t header = block->header;
-        fprintf(file, "  Header position: %" PRId64 " (0x%" PRIx64 ")\n",
-                (int64_t)block->header_pos, (int64_t)block->header_pos);
-        fprintf(file, "  Data position: %" PRId64 " (0x%" PRIx64 ")\n",
-                (int64_t)block->data_pos, (int64_t)block->data_pos);
-        fprintf(file, "  Allocated size: %" PRIu64 " (0x%" PRIx64 ")\n",
-                header.allocated_size, header.allocated_size);
-        fprintf(file, "  Used size: %" PRIu64 " (0x%" PRIx64 ")\n", header.used_size,
-                header.used_size);
-        fprintf(file, "  Data size: %" PRIu64 " (0x%" PRIx64 ")\n", header.data_size,
-                header.data_size);
+        fprintf(
+            file,
+            "  Header position: %" PRId64 " (0x%" PRIx64 ")\n",
+            (int64_t)block->header_pos,
+            (int64_t)block->header_pos);
+        fprintf(
+            file,
+            "  Data position: %" PRId64 " (0x%" PRIx64 ")\n",
+            (int64_t)block->data_pos,
+            (int64_t)block->data_pos);
+        fprintf(
+            file,
+            "  Allocated size: %" PRIu64 " (0x%" PRIx64 ")\n",
+            header.allocated_size,
+            header.allocated_size);
+        fprintf(
+            file, "  Used size: %" PRIu64 " (0x%" PRIx64 ")\n", header.used_size, header.used_size);
+        fprintf(
+            file, "  Data size: %" PRIu64 " (0x%" PRIx64 ")\n", header.data_size, header.data_size);
 
         if (header.compression[0] != '\0')
             fprintf(file, "  Compression: %.4s\n", header.compression);
@@ -152,9 +176,6 @@ void asdf_event_destroy(asdf_parser_t *parser, asdf_event_t *event) {
     switch (event->type) {
     case ASDF_TREE_START_EVENT:
     case ASDF_TREE_END_EVENT:
-        if (event->payload.tree)
-            free(event->payload.tree->buf);
-
         free(event->payload.tree);
         break;
     case ASDF_YAML_EVENT:

src/event.h

@@ -37,7 +37,7 @@ typedef enum {
 } asdf_event_type_t;
 
 
-static const char *const ASDF_EVENT_TYPE_NAMES[] = {
+static const char *const asdf_event_type_names[] = {
 #define X(name) #name,
     ASDF_EVENT_TYPES(X)
 #undef X
@@ -52,10 +52,13 @@ typedef struct {
 typedef struct {
     size_t start;
     size_t end;
-    char *buf;
+    const char *buf;
 } asdf_tree_info_t;
 
 
+typedef struct fy_event asdf_yaml_event_t;
+
+
 typedef struct asdf_event {
     asdf_event_type_t type;
     union {

src/info.c

@@ -449,25 +449,24 @@ void print_block(FILE *file, const asdf_event_t *event, size_t block_idx) {
 }
 
 
-static const asdf_info_cfg_t ASDF_INFO_DEFAULT_CFG = {
+static const asdf_info_cfg_t asdf_info_default_cfg = {
     .filename = NULL, .print_tree = true, .print_blocks = false};
 
 
 int asdf_info(FILE *in_file, FILE *out_file, const asdf_info_cfg_t *cfg) {
     asdf_parser_t parser = {0};
 
     if (!cfg)
-        cfg = &ASDF_INFO_DEFAULT_CFG;
+        cfg = &asdf_info_default_cfg;
 
     // Current implementation needs YAML events unless no-tree
     asdf_parser_cfg_t parser_cfg = {
-        .flags = cfg->print_tree ? ASDF_PARSER_OPT_EMIT_YAML_EVENTS : 0
-    };
+        .flags = cfg->print_tree ? ASDF_PARSER_OPT_EMIT_YAML_EVENTS : 0};
 
     if (asdf_parser_init(&parser, &parser_cfg) != 0)
         return 1;
 
-    if (asdf_parser_set_input_file(&parser, in_file, cfg->filename) != 0) {
+    if (asdf_parser_set_input_fp(&parser, in_file, cfg->filename) != 0) {
         asdf_parser_destroy(&parser);
         return 1;
     }
@@ -500,6 +499,14 @@ int asdf_info(FILE *in_file, FILE *out_file, const asdf_info_cfg_t *cfg) {
         }
     }
 
+    if (asdf_parser_has_error(&parser)) {
+        const char *error = asdf_parser_get_error(&parser);
+        // TODO: (#5) Better error formatting / probably go through logging system
+        fprintf(stderr, "error: %s\n", error);
+        asdf_parser_destroy(&parser);
+        return 1;
+    }
+
     asdf_parser_destroy(&parser);
     return 0;
 }