The av2notification_meta() function updated it's parameter by doing
(*m) = malloc();
Afterwards, if an error occurred, it would call `free(*m);` and (potentially)
return, leaving an invalid pointer stored in `m`. The caller would then try to
free the returned pointer because it was non-NULL.
This patch fixes this by doing several code cleanups:
* The meta argument is not updated unnecessarily. Instead, a local variable
is allocated and used within the loop and only on success is the return
pointer updated.
* Introduce additional local variables instead of re-using the `tmp` variable
multiple times.
* Name the variable pointing to the end of the linked list appropriately
`tail` and remove one level of indirection. Reading code that is using
pointers to pointers is unnecessarily hard.
* meta => [ { name => <name>, value => <value> }, ... ]
* }
*/
* meta => [ { name => <name>, value => <value> }, ... ]
* }
*/
-static int av2notification_meta(pTHX_ AV *array, notification_meta_t **meta) {
- notification_meta_t **m = meta;
+static int av2notification_meta(pTHX_ AV *array, notification_meta_t **ret_meta) {
+ notification_meta_t *tail = NULL;
int len = av_len(array);
for (int i = 0; i <= len; ++i) {
SV **tmp = av_fetch(array, i, 0);
int len = av_len(array);
for (int i = 0; i <= len; ++i) {
SV **tmp = av_fetch(array, i, 0);
return -1;
if (!(SvROK(*tmp) && (SVt_PVHV == SvTYPE(SvRV(*tmp))))) {
return -1;
if (!(SvROK(*tmp) && (SVt_PVHV == SvTYPE(SvRV(*tmp))))) {
- hash = (HV *)SvRV(*tmp);
+ HV *hash = (HV *)SvRV(*tmp);
- *m = smalloc(sizeof(**m));
+ notification_meta_t *m = calloc(1, sizeof(*m));
+ if (m == NULL)
+ return ENOMEM;
- if (NULL == (tmp = hv_fetch(hash, "name", 4, 0))) {
+ SV **name = hv_fetch(hash, "name", strlen("name"), 0);
+ if (name == NULL) {
log_warn("av2notification_meta: Skipping invalid "
"meta information.");
log_warn("av2notification_meta: Skipping invalid "
"meta information.");
- sstrncpy((*m)->name, SvPV_nolen(*tmp), sizeof((*m)->name));
+ sstrncpy(m->name, SvPV_nolen(*name), sizeof(m->name));
- if (NULL == (tmp = hv_fetch(hash, "value", 5, 0))) {
+ SV **value = hv_fetch(hash, "value", strlen("value"), 0);
+ if (value == NULL) {
log_warn("av2notification_meta: Skipping invalid "
"meta information.");
log_warn("av2notification_meta: Skipping invalid "
"meta information.");
- if (SvNOK(*tmp)) {
- (*m)->nm_value.nm_double = SvNVX(*tmp);
- (*m)->type = NM_TYPE_DOUBLE;
- } else if (SvUOK(*tmp)) {
- (*m)->nm_value.nm_unsigned_int = SvUVX(*tmp);
- (*m)->type = NM_TYPE_UNSIGNED_INT;
- } else if (SvIOK(*tmp)) {
- (*m)->nm_value.nm_signed_int = SvIVX(*tmp);
- (*m)->type = NM_TYPE_SIGNED_INT;
+ if (SvNOK(*value)) {
+ m->nm_value.nm_double = SvNVX(*value);
+ m->type = NM_TYPE_DOUBLE;
+ } else if (SvUOK(*value)) {
+ m->nm_value.nm_unsigned_int = SvUVX(*value);
+ m->type = NM_TYPE_UNSIGNED_INT;
+ } else if (SvIOK(*value)) {
+ m->nm_value.nm_signed_int = SvIVX(*value);
+ m->type = NM_TYPE_SIGNED_INT;
- (*m)->nm_value.nm_string = sstrdup(SvPV_nolen(*tmp));
- (*m)->type = NM_TYPE_STRING;
+ m->nm_value.nm_string = sstrdup(SvPV_nolen(*value));
+ m->type = NM_TYPE_STRING;
- (*m)->next = NULL;
- m = &((*m)->next);
+ m->next = NULL;
+ if (tail == NULL)
+ *ret_meta = m;
+ else
+ tail->next = m;
+ tail = m;
return 0;
} /* static int av2notification_meta (AV *, notification_meta_t *) */
return 0;
} /* static int av2notification_meta (AV *, notification_meta_t *) */