Add functions to insert pods/containers to the new trees - #381
Conversation
c664569 to
303ae97
Compare
biscout42
left a comment
biscout42
left a comment
There was a problem hiding this comment.
My main ask if we have unit tests or could we add them?
| if (pod_uid != NULL) { | ||
| pod = pod_lookup_by_uid(qq, (char *)pod_uid); | ||
| if (pod == NULL) | ||
| return (NULL); |
There was a problem hiding this comment.
Question: how do we usually report errors in C? IMHO this case is kind of pod not found error. WDYT?
| pod->uid = strdup(uid); | ||
| if (pod->uid == NULL) | ||
| goto fail; | ||
| if (name != NULL && (pod->name = strdup(name)) == NULL) |
There was a problem hiding this comment.
Bot says, if we pass name, nsorpahseasNULL, we will have a pod inserted into the tree, which will fail quark_event_dump()`.
Would it make sense to add a guard and reject inserting null values?
There was a problem hiding this comment.
Do we have a uni test for quark_pod_create? We could do a round trip test with create / get
| } | ||
|
|
||
| struct quark_container * | ||
| quark_container_create(struct quark_queue *qq, const char *container_id, |
There was a problem hiding this comment.
Do we have a uni test for quark_container_create? We could do a round trip test with create / get
| container->container_id = strdup(container_id); | ||
| if (container->container_id == NULL) | ||
| goto fail; | ||
| if (name != NULL && (container->name = strdup(name)) == NULL) |
There was a problem hiding this comment.
same here, bot says quark_event_dump() will fail with this fields being NULL. Shall we prevent it?
2 participants
Stack created with GitHub Stacks CLI • Give Feedback 💬