Skip to content

Commit

Permalink
Revert "These changes are a contribution from Robert Boisvert and the…
Browse files Browse the repository at this point in the history
… Alpine Linux team to address an issue that was encountered while testing with valgrind. The reported problem was that of an 'uninitialized value', and Robert Boisvert's statement was that the changes came from 'upstream'... although clearly he was not referring to upstream spandsp proper. By 'upstream' he may simply have been reporting what was told to him by others, and 'upstream' may actually mean some other fork of spandsp somewhere. That said, the claim was that this change resolved the issue as reported by valgrind. I'm not sure why the code needed to be so refactored as it was in order to resolve an uninitialized value, and as many times as I've examined this I've never really understood the uninitialized value issue here. However, admittedly the changed code is easier to follow, and as at_cmd_plus_VSID() was the only function to utilize parse_string_out(), I ultimately felt good about adopting it."

This reverts commit a701c59.

Discussion with Steve Underwood indicates that whatever the uninitialized value issue was here, it has been remedied previously, and the code refactoring is undesireable.
  • Loading branch information
redder86 committed Jun 9, 2022
1 parent 461bf79 commit 056a9e6
Showing 1 changed file with 40 additions and 24 deletions.
64 changes: 40 additions & 24 deletions src/at_interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,43 @@ static int parse_string_list_out(at_state_t *s, const char **t, int *target, int
}
/*- End of function --------------------------------------------------------*/

static int parse_string_out(at_state_t *s, const char **t, char **target, const char *prefix)
{
char buf[100];

switch (*(*t)++)
{
case '=':
switch (**t)
{
case '?':
/* Show possible values */
(*t)++;
snprintf(buf, sizeof(buf), "%s", (prefix) ? prefix : "");
at_put_response(s, buf);
break;
default:
/* Set value */
if (*target)
span_free(*target);
/* If this strdup fails, it should be harmless */
*target = strdup(*t);
break;
}
break;
case '?':
/* Show current index value */
at_put_response(s, (*target) ? *target : "");
break;
default:
return false;
}
while (**t)
(*t)++;
return true;
}
/*- End of function --------------------------------------------------------*/

static const char *s_reg_handler(at_state_t *s, const char *t, int reg)
{
int val;
Expand Down Expand Up @@ -5012,31 +5049,10 @@ static const char *at_cmd_plus_VSID(at_state_t *s, const char *t)
{
/* Extension of V.253 +VCID, Set calling number ID */
t += 5;
switch (*t)
{
case '=':
switch (*(t+1))
{
case '?':
/* Show possible values */
at_put_response(s, "");
break;
default:
/* Set value */
s->local_id = strdup(t + 1);
if (at_modem_control(s, AT_MODEM_CONTROL_SETID, s->local_id) < 0)
return NULL;
break;
}
break;
case '?':
/* Show current index value */
at_put_response(s, (s->local_id) ? s->local_id : "");
break;
default:
if (!parse_string_out(s, &t, &s->local_id, NULL))
return NULL;
if (at_modem_control(s, AT_MODEM_CONTROL_SETID, s->local_id) < 0)
return NULL;
}
while (*t) t++;
return t;
}
/*- End of function --------------------------------------------------------*/
Expand Down

1 comment on commit 056a9e6

@coppice-git
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Committed.

Please sign in to comment.