View Issue Details

IDProjectCategoryView StatusLast Update
0005459Talerexchangepublic2019-12-20 18:12
ReporterMarcello Stanisci Assigned ToMarcello Stanisci Resolution deadline
ImplementerReviewer 
Release signed byDeployed by 
VerifierCommunicator 
Data protection authority notification requiredNo  
CSIRT notification requiredNo  
Estimated effort (hours)Actual effort (hours)0 
Fast-trackNo
Work packagenone
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.

Required notifications

Address or linkRemarkAdded byNotifiedActions

Activities

Marcello Stanisci

2018-12-19 09: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 09: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 10: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 11: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 11: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 12: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 ModifiedUsernameFieldChange
2018-10-15 10:10Marcello StanisciNew Issue
2018-10-15 10:10Marcello StanisciStatusnew => assigned
2018-10-15 10:10Marcello StanisciAssigned To => Christian Grothoff
2018-10-15 10:11Marcello StanisciAssigned ToChristian Grothoff => Marcello Stanisci
2018-11-17 23:37Christian GrothoffSeverityminor => tweak
2018-11-17 23:37Christian GrothoffProduct Version => git (master)
2018-11-17 23:37Christian GrothoffTarget Version => 0.8
2018-12-19 09:45Marcello StanisciNote Added: 0013420
2018-12-19 09:50Christian GrothoffNote Added: 0013421
2018-12-19 10:00Marcello StanisciNote Added: 0013422
2018-12-19 11:31Christian GrothoffNote Added: 0013423
2018-12-19 11:44Marcello StanisciStatusassigned => informing
2018-12-19 11:44Marcello StanisciResolutionopen => fixed
2018-12-19 11:44Marcello StanisciNote Added: 0013424
2018-12-19 12:02Christian GrothoffNote Added: 0013425
2018-12-19 12:02Christian GrothoffFixed in Version => 0.6
2018-12-19 12:02Christian GrothoffTarget Version0.8 => 0.6
2019-12-20 18:12Christian GrothoffStatusinforming => closed