Attention is currently required from: Thomas Heijligen, Edward O'Callaghan.

Patch set 1:Code-Review +1

View Change

7 comments:

• Patchset:

  • Patch Set #1:

    Interesting idea! It looks nice, although coding style is rather inconsistent.

• File util/flashchips_db_jsoniser/main.c:

  • Patch Set #1, Line 65: ret = realloc(ret, strlen(ret) + 1);

    Is this needed?

  • Patch Set #1, Line 69:

    static char *decode_test_state(enum test_state ts)
    {
      switch (ts) {
        case OK: return "OK";
        case NT: return "NT";
        case BAD: return "BAD";
        case DEP: return "DEP";
        case NA: return "NA";
      }
      return NULL;
    };
    
    static char *decode_cmdset(int cmdset)
    {
      switch (cmdset) {
        case SPI25: return "SPI25";
        case SPI_EDI: return "SPI_EDI";
      }
      return NULL;
    }
    

    Indentation: please use tabs instead of spaces

  • Patch Set #1, Line 113: return NULL;

    Indentation

  • Patch Set #1, Line 162:

      printf("\t\t\t\t\"block_erasers\": [\n");
    
      for (unsigned i = 0; i < NUM_ERASEFUNCTIONS; i++) {
        const struct block_eraser *be = &bes[i];
    
        if (i) printf(",\n");
        printf("\t\t\t\t\t{\n");
        printf("\t\t\t\t\t\t\"%d\": [\n", i);
        for (unsigned j = 0; j < NUM_ERASEREGIONS; j++) {
          struct eraseblock block = be->eraseblocks[j];
          if (j) printf(",\n");
          printf("\t\t\t\t\t\t\t{\n");
          printf("\t\t\t\t\t\t\t\t\"size\": %d,\n", block.size);
          printf("\t\t\t\t\t\t\t\t\"count\": %d\n", block.size);
          printf("\t\t\t\t\t\t\t}");
        }
        printf("\t\t\t\t\t\t],\n");
    
        enum block_erase_func block_erase = be->block_erase;
        const char *block_erase_type = decode_block_eraser_type(block_erase);
        printf("\t\t\t\t\t\t\"block_erase\": \"%s\"\n", block_erase_type);
    
        printf("\t\t\t\t\t}\n");
      }
    
      printf("\t\t\t\t],\n");
    

    Indentation

  • Patch Set #1, Line 194: case WRITE_JEDEC1: return "WRITE_JEDEC_1";

    Indentation (this one uses 8 spaces)

  • Patch Set #1, Line 228:

      printf("{\n\t\t\"flashchips\": [\n");
    
      for (unsigned int i = 0; i < flashchips_size; i++) {
        if (i) printf(",\n");
        printf("\t\t\t{\n");
    
        printf("\t\t\t\t\"vendor\": \"%s\",\n", flashchips[i].vendor);
        printf("\t\t\t\t\"name\": \"%s\",\n", flashchips[i].name);
    
        // bustype
        char *bustype = flashbuses_to_text_copy(flashchips[i].bustype);
        printf("\t\t\t\t\"bustype\": \"%s\",\n", bustype);
        free(bustype);
    
        printf("\t\t\t\t\"manufacture_id\": %d,\n", flashchips[i].manufacture_id);
        printf("\t\t\t\t\"model_id\": %d,\n", flashchips[i].model_id);
        printf("\t\t\t\t\"total_size\": %d,\n", flashchips[i].total_size);
        printf("\t\t\t\t\"page_size\": %d,\n", flashchips[i].page_size);
        printf("\t\t\t\t\"feature_bits\": %d,\n", flashchips[i].feature_bits);
    
        // tested
        char *pts = decode_test_state(flashchips[i].tested.probe);
        char *rts = decode_test_state(flashchips[i].tested.read);
        char *ets = decode_test_state(flashchips[i].tested.erase);
        char *wts = decode_test_state(flashchips[i].tested.write);
        char *wpts = decode_test_state(flashchips[i].tested.wp);
        printf("\t\t\t\t\"test_state\": [\n");
        printf("\t\t\t\t\t{ \"probe\": \"%s\"},\n", pts);
        printf("\t\t\t\t\t{ \"read\": \"%s\"},\n", rts);
        printf("\t\t\t\t\t{ \"erase\": \"%s\"},\n", ets);
        printf("\t\t\t\t\t{ \"write\": \"%s\"},\n", wts);
        printf("\t\t\t\t\t{ \"wp\": \"%s\"}\n", wpts);
        printf("\t\t\t\t],\n");
    
        // spi_cmd_set
        char *cmdset = decode_cmdset(flashchips[i].spi_cmd_set);
        printf("\t\t\t\t\"spi_cmd_set\": \"%s\",\n", cmdset);
    
        // probe_func probe;
        char *probefn = decode_probefn(flashchips[i].probe);
        printf("\t\t\t\t\"probe\": \"%s\",\n", probefn);
    
        printf("\t\t\t\t\"probe_timing\": %d,\n", flashchips[i].probe_timing);
    
        // block_erasers[NUM_ERASEFUNCTIONS];
        decode_block_erasers(flashchips[i].block_erasers);
    
        // enum write_func write;
        const char *writefn = decode_writefn(flashchips[i].write);
        printf("\t\t\t\t\"write\": \"%s\",\n", writefn);
    
        // enum read_func read;
        const char *readfn = decode_readfn(flashchips[i].read);
        printf("\t\t\t\t\"read\": \"%s\",\n", readfn);
    
        // struct voltage {
        //   uint16_t min;
        //   uint16_t max;
        // } voltage;
        printf("\t\t\t\t\"voltage\": [\n");
        printf("\t\t\t\t\t{ \"min\": %d },\n", flashchips[i].voltage.min);
        printf("\t\t\t\t\t{ \"max\": %d }\n", flashchips[i].voltage.max);
        printf("\t\t\t\t]\n");
    
        // enum write_granularity gran;
    
        printf("\n\t\t\t}\n");
      }
    
      printf("\t]\n}");
      return 0;
    

    Indentation

To view, visit change 74065. To unsubscribe, or for help writing mail filters, visit settings.