Hi Max,

thanks for looking at the msc_vlr_tests.c. That 'while (0)' was a curious
artifact indeed :)

Some parts of it I actually wouldn't have liked to be merged:

1) pass nr as param / always print the test number.

The reason why I wanted that number output only in verbose mode is, I don't
want to adjust unrelated *.err output when adding a test in any position that
is not the last. The test nr was introduced only for running a specific test
manually, and for that the -v option provides the test number to whoever is
invoking it manually.

The individual tests do not at all need to know their number, so I would prefer
not passing it as parameter. The idea is to keep the manual invocation overhead
out of the production testing code.

2) pass IMSI as param / print the IMSI.

The IMSI used in the tests is (incidentally) the same throughout the tests and
doesn't really need to change in any way. It is but a local variable that
ensures there are no string constant typos within a test function.  It mayybe
could be one or more global string constants, but doesn't make sense as a
parameter passed to each and every test: some tests also (might) use two IMSIs.
Running the same tests with differing IMSIs is not needed, and I don't see a
patch introducing that. What was the idea there to justify the bloat?

(If we were to pass imsi and number to each and every msc_vlr_test* function,
it would be better to introduce a context struct that we can modify without
having to edit every function signature. But I'd prefer them parameter-less.)

3) you moved gsm_network creation into a separate function, but that function
is still called only once. So it's just cosmetic, or was there another purpose?


Attachment: signature.asc
Description: PGP signature

Reply via email to