Hi.
I don't think that remove_source() in sigrok-cli.c works as intended.
1. It always allocates the same amount of memory that is already in use,
i.e. num_sources not num_sources - 1.
2. It doesn't do the right thing if there is only one source to remove.
In the attached patch, I've rewritten it to not do any dynamic allocation at
all, it just shrinks the already-allocated array with the common trick of
copying the last element to the vacated slot.
I think this would be a perfect place to use glib's dynamic array (to cut
down on the malloc()/free() dancing), but I gather that there's an intention
to move sigrok away from glib.
Feedback welcome, as always.
Regards,
/Emil
diff --git a/cli/sigrok-cli.c b/cli/sigrok-cli.c
index 9e4d396..2ee504d 100644
--- a/cli/sigrok-cli.c
+++ b/cli/sigrok-cli.c
@@ -536,25 +536,22 @@ uint64_t parse_sizestring(char *sizestring)
void remove_source(int fd)
{
- struct source *new_sources;
- int old, new;
+ int i;
if (!sources)
return;
- new_sources = calloc(1, sizeof(struct source) * num_sources);
- for (old = 0; old < num_sources; old++)
- if (sources[old].fd != fd)
- memcpy(&new_sources[new++], &sources[old],
- sizeof(struct source));
-
- if (old != new) {
- free(sources);
- sources = new_sources;
- num_sources--;
- } else {
- /* Target fd was not found. */
- free(new_sources);
+ for (i = 0; i < num_sources; i++)
+ {
+ if (sources[i].fd == fd)
+ {
+ if (i + 1 < num_sources)
+ {
+ sources[i] = sources[num_sources - 1];
+ }
+ num_sources--;
+ break;
+ }
}
}
------------------------------------------------------------------------------
_______________________________________________
sigrok-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
