Re: [PATCH 2/3 v3] annotations: add positions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



> > +struct srcpos *srcpos_extend(struct srcpos *pos, struct srcpos *newtail)
> > +{
> > +	struct srcpos *next, *p;
> > +
> > +	if (!pos)
> > +		return newtail;
>
> I'm having a lot of trouble following this.  You've got both a
> recursion and a loop through the list, so you seem to have an O(n^2)
> with multiple redundant checks.
>
> > +
> > +	next = srcpos_extend(pos->next, newtail);
> > +
> > +	for (p = next; p != NULL; p = p->next)
> > +		if (same_pos(pos, p))
> > +			return next;
> > +
> > +	pos = srcpos_copy(pos);
>
> And while I'm not *that* fussed by memory leaks, I have a nasty
> feeling this will leak stuff at every level of the recursion, which is
> a bit much.
>
> > +	pos->next = next;
> > +	return pos;
> > +}

It copies the first list and puts the second list at the end of it.  The
inner for loop checks that the current element in the first list is not
found in the rest of the constructed list.  It should be posssible to
replace:

for (p = next; p != NULL; p = p->next)

by

for (p = newtail; p != NULL; p = p->next)

since the original lists should not have duplicates.

Just overwriting the next field at the end of the first list to point to
the second list creates infinite loops.

julia

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux