View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0005459Talerexchangepublic2018-10-15 12:102019-12-20 19:12
ReporterMarcello Stanisci Assigned ToMarcello Stanisci  
PrioritynormalSeveritytweakReproducibilityhave not tried
Status closedResolutionfixed 
Product Versiongit (master) 
Target Version0.6Fixed in Version0.6 
Summary0005459: Change the way CMD structs are defined.
DescriptionEvery testing_api_cmd_*.c file should initialize their 'struct TALER_TESTING_Command' objects in the following way:

TYPE cmd {

 .field1 = value;
 .field2 = value;
};

This style enforces all those values not mentioned in the
initialization to be created as 0/NULL. OTOH, the current
style leave UNinitialized those fields that do not explicitly
get a value.
Additional InformationAlthough this bug targets the exchange, such changes should be carried on the merchant's, twister's (and possibly other?) codebases.
TagsNo tags attached.

Activities

Marcello Stanisci

2018-12-19 10:45

reporter   ~0013420

You sure mean 'struct TALER_TESTING_Command'? There are also the various 'struct CommandXState' types; looks like they can also benefit from this change.

Christian Grothoff

2018-12-19 10:50

manager   ~0013421

Likely that other structs could also benefit. And I did write 'struct TALER_TESTING_Command', so I'm not sure what your question is.

Example: testing_api_cmd_wire.c:

  struct TALER_TESTING_Command cmd;
  struct WireState *ws;

  ws = GNUNET_new (struct WireState);
  ws->exchange = exchange;
  ws->expected_method = expected_method;
  ws->expected_fee = expected_fee;
  ws->expected_response_code = expected_response_code;

  cmd.cls = ws;
  cmd.label = label;
  cmd.run = &wire_run;
  cmd.cleanup = &wire_cleanup;


could be:

  struct TALER_TESTING_Command cmd = {
    .label = label,
    .run = &wire_run,
    .cleanup = &wire_cleanup
  };

  struct WireState *ws;

  ws = GNUNET_new (struct WireState);
  ws->exchange = exchange;
  ws->expected_method = expected_method;
  ws->expected_fee = expected_fee;
  ws->expected_response_code = expected_response_code;

  cmd.cls = ws;
  return cmd;
}

Marcello Stanisci

2018-12-19 11:00

reporter   ~0013422

Is it okay to put it the following way? I think it's better because you don't have to touch 'cmd' twice.

struct WireState *ws;

ws = GNUNET_new (struct WireState);
ws->exchange = exchange;
ws->expected_method = expected_method;
ws->expected_fee = expected_fee;
ws->expected_response_code = expected_response_code;

struct TALER_TESTING_Command cmd = {
  .label = label,
  .run = &wire_run,
  .cleanup = &wire_cleanup,
  .cls = ws
};
  
return cmd;

Christian Grothoff

2018-12-19 12:31

manager   ~0013423

Kind-of. If we want to be C89-compatible, you should start a new scope, i.e.:

{ // NEW!
  struct TALER_TESTING_Command cmd = {
    .label = label,
    .run = &wire_run,
    .cleanup = &wire_cleanup,
    .cls = ws
  };
  
  return cmd;
} // NEW!

Marcello Stanisci

2018-12-19 12:44

reporter   ~0013424

I did "my" (non-c89-compatible) way for now. The three codebases got it at the following commits:

exchange: 9c82290a
merchant: 6b184e2
twister: 396b774

I also could not (quickly) find any documentation about this c89 style.

Let me know if I shall correct it to follow c89.

Christian Grothoff

2018-12-19 13:02

manager   ~0013425

Nah, we don't generally require c89 for GNUnet or Taler (only C99, where what you do is fine). Still, I do like scope minimization, but in this case I guess that might indeed be overkill.

Issue History

Date Modified Username Field Change
2018-10-15 12:10 Marcello Stanisci New Issue
2018-10-15 12:10 Marcello Stanisci Status new => assigned
2018-10-15 12:10 Marcello Stanisci Assigned To => Christian Grothoff
2018-10-15 12:11 Marcello Stanisci Assigned To Christian Grothoff => Marcello Stanisci
2018-11-18 00:37 Christian Grothoff Severity minor => tweak
2018-11-18 00:37 Christian Grothoff Product Version => git (master)
2018-11-18 00:37 Christian Grothoff Target Version => 0.8
2018-12-19 10:45 Marcello Stanisci Note Added: 0013420
2018-12-19 10:50 Christian Grothoff Note Added: 0013421
2018-12-19 11:00 Marcello Stanisci Note Added: 0013422
2018-12-19 12:31 Christian Grothoff Note Added: 0013423
2018-12-19 12:44 Marcello Stanisci Status assigned => resolved
2018-12-19 12:44 Marcello Stanisci Resolution open => fixed
2018-12-19 12:44 Marcello Stanisci Note Added: 0013424
2018-12-19 13:02 Christian Grothoff Note Added: 0013425
2018-12-19 13:02 Christian Grothoff Fixed in Version => 0.6
2018-12-19 13:02 Christian Grothoff Target Version 0.8 => 0.6
2019-12-20 19:12 Christian Grothoff Status resolved => closed