Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk.

View Change

15 comments:

Commit Message:
- Patch Set #6, Line 7: Convert
  Replace Convert with Add. […]
  Done
File board_enable.c:
- Patch Set #6, Line 2527: const size_t board_matches_size = ARRAY_SIZE(board_matches);
  Why this can't be in the test code?
  the size of the array is only known in the translation unit it is defined in
File tests/meson.build:
- Patch Set #6, Line 25: well_formed
  I would rename all occurrences of "well_formed" into "selfcheck": file names, test methods names etc […]
  Done
File tests/well_formed_tables.c:
- Patch Set #6, Line 23: name
  Maybe rename to prog_name?
  its only prog_name for one of the callsites
- Patch Set #6, Line 26: is false
  is false or NULL
  Done
- Patch Set #6, Line 26: #assertion
  I assume this would print "p is false" which can be confusing. […]
  #assertion is copied from what normal cmocka assertions print. We can do better.
- Patch Set #6, Line 24:
```
	do {                                                                                   \
		if (!(assertion))                                                              \
			fail_msg(#assertion " is false for index:%zu name:%s", (index),        \
				 (name) ? (name) : "unknown");                                 \
	} while (0)
```
  I was also confused in the beginning, but then I thought this is probably a trick to avoid swallowin […]
  yes its the standard semicolon trick for macros. So you can call it like a function.
- Patch Set #6, Line 33: (void)state; /* unused */
  Add a new line after `(void)state; /* unused */` and the same for all other test methods.
  Done
- Patch Set #6, Line 41:
```
if (strcmp("internal", p->name) != 0)
			assert_table(p->devs.note, i, p->name);
```
  Original code has this check for all programmers with p->type == OTHER […]
  the original code does this check for all types except 'default', and for default it does an unconditional assertion. I replaced that assertion with assert(p->type), although that is not as good as it no longer checks for non-valid values of the enum. I added that back in.
- Patch Set #6, Line 51: assert_false(flashchips_size <= 1 || flashchips[flashchips_size - 1].name != NULL);
  Break down into two `assert_true`s
  Done
- Patch Set #6, Line 58: }
  You missed `selfcheck_eraseblocks` functionality? […]
  Done
- Patch Set #6, Line 66:
```
assert_false(board_matches_size <= 1
		     || board_matches[board_matches_size - 1].vendor_name != NULL);
```
  Break down into two `assert_true`s
  Done
- Patch Set #6, Line 71:
```
		assert_table(b->board_name, i, b->board_name);
		assert_table(b->board_name, i, b->board_name);
```
  The same line twice. […]
  Done
- Patch Set #6, Line 83: SKIP_TEST
  Indent with tab
  Done
- Patch Set #6, Line 84: // CONFIG_INTERNAL
  /* comment */
  Done

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