Hi Jose,
> diff --git a/lib/sdp.c b/lib/sdp.c
> index 822ec1a..ddf2d1a 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -4621,3 +4621,113 @@ uint16_t sdp_gen_tid(sdp_session_t *session)
> {
> return session->tid++;
> }
> +
> +/*
> + * Set the supported features
> + */
> +int sdp_set_supp_feat(sdp_record_t *rec, const sdp_list_t *sf)
> +{
> + const sdp_list_t *p, *r;
> + sdp_data_t *feat, *seq_feat;
> + int seqlen, i, err;
> + void **seqDTDs, **seqVals;
> +
> + err = -1;
what is this err = -1 business. That is pointless. See below.
> + i = 0;
Move this to the place where it is actually used.
> + seqlen = sdp_list_len(sf);
> + seqDTDs = malloc(seqlen * sizeof(void *));
> + if (!seqDTDs)
> + return err;
This is return -1;
> + seqVals = malloc(seqlen * sizeof(void *));
> + if (!seqVals) {
> + free(seqDTDs);
> + return err;
And this, too. Just use return -1;
> + }
> +
> + for (p = sf; p; p = p->next) {
> + int plen, j;
> + void **dtds, **vals;
> +
> + plen = sdp_list_len(p->data);
> + dtds = malloc(plen * sizeof(void *));
> + if (!dtds)
> + goto set_sup_feat_exit;
> + vals = malloc(plen * sizeof(void *));
> + if (!vals) {
> + free(dtds);
> + goto set_sup_feat_exit;
> + }
> + j = 0;
> + for (r = p->data; r; r = r->next) {
> + sdp_data_t *data = (sdp_data_t*)r->data;
> + dtds[j] = &data->dtd;
> + vals[j] = &data->val;
> + j++;
> + }
> + feat = sdp_seq_alloc(dtds, vals, plen);
> + free(dtds);
> + free(vals);
> + if (!feat)
> + goto set_sup_feat_exit;
> + seqDTDs[i] = &feat->dtd;
> + seqVals[i] = feat;
> + i++;
> + }
> + seq_feat = sdp_seq_alloc(seqDTDs, seqVals, seqlen);
> + if (!seq_feat)
> + goto set_sup_feat_exit;
> + sdp_attr_replace(rec, SDP_ATTR_SUPPORTED_FEATURES_LIST, seq_feat);
> +
> + err = 0;
Replace this with return 0;
> +
> +set_sup_feat_exit:
Call this label "fail" to be consistent across the code.
> + free(seqVals);
> + free(seqDTDs);
> + return err;
This needs to be return -1;
And then remove the whole err variable since I don't see it used
anywhere.
> +}
> +
> +/*
> + * Get the supported features
> + * If an error occurred -1 is returned and errno is set
> + */
> +int sdp_get_supp_feat(const sdp_record_t *rec, sdp_list_t **seqp)
> +{
> + sdp_data_t * sdpdata, *d;
> + sdp_list_t * next;
Who said we declare variable like this. It is sdp_list_t *next. No extra
space between * and the variable name.
> + *seqp = NULL;
This is pretty much a bad idea. So in case this functions fails you
should not touch *seqp at all. It is not your pointer in the end.
I would prefer if you use a temporary pointer and only assign it on
success.
> + sdpdata = sdp_data_get(rec, SDP_ATTR_SUPPORTED_FEATURES_LIST);
> +
> + if (!sdpdata || sdpdata->dtd < SDP_SEQ8 || sdpdata->dtd > SDP_SEQ32)
> + return sdp_get_uuidseq_attr(rec,
> + SDP_ATTR_SUPPORTED_FEATURES_LIST, seqp);
> +
> + for (d = sdpdata->val.dataseq; d; d = d->next) {
> + sdp_data_t *dd;
> + sdp_list_t *subseq;
> + subseq = NULL;
> + if (d->dtd < SDP_SEQ8 || d->dtd > SDP_SEQ32)
> + goto fail;
Move the subseq after the if check.
> + for (dd = d->val.dataseq; dd; dd = dd->next) {
> + sdp_data_t *data;
> + if (dd->dtd != SDP_UINT8 && dd->dtd != SDP_UINT16 &&
> + dd->dtd != SDP_TEXT_STR8)
> + goto fail;
> + data = sdp_data_alloc(dd->dtd, &dd->val);
> + if (data)
> + subseq = sdp_list_append(subseq, data);
> + }
> + *seqp = sdp_list_append(*seqp, subseq);
> + }
> + return 0;
> +
> +fail:
> + while (*seqp) {
> + next = (*seqp)->next;
The next variable could be declared inside the while loop actually.
> + sdp_list_free(*seqp, free);
> + *seqp = next;
> + }
> + errno = EINVAL;
> + return -1;
> +}
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
