-
Notifications
You must be signed in to change notification settings - Fork 27
changed addReccasterEnvVars to linked list implementation #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
5f43a89
f93c0cf
925515d
5836a6d
e87e20f
9bc594a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,7 +74,6 @@ static void casthook(initHookState state) | |
| void addReccasterEnvVars(caster_t* self, int argc, char **argv) | ||
| { | ||
| size_t i, j; | ||
| int ret = 0; | ||
|
|
||
| argv++; argc--; /* skip function arg */ | ||
| if(argc < 1) { | ||
|
|
@@ -95,91 +94,54 @@ void addReccasterEnvVars(caster_t* self, int argc, char **argv) | |
| epicsMutexUnlock(self->lock); | ||
| return; | ||
| } | ||
| int new_extra_envs_size = self->num_extra_envs + argc; | ||
| int num_new_extra_envs = self->num_extra_envs; | ||
|
|
||
| char **new_extra_envs = calloc(new_extra_envs_size, sizeof(*new_extra_envs)); | ||
| if(new_extra_envs == NULL) { | ||
| errlogSevPrintf(errlogMajor, "Error in memory allocation of new_extra_envs from addReccasterEnvVars\n"); | ||
| epicsMutexUnlock(self->lock); | ||
| return; | ||
| } | ||
| /* copy self->extra_envs into new_extra_envs with room for new envs */ | ||
| for(i=0; i < self->num_extra_envs; i++) { | ||
| if((new_extra_envs[i] = strdup(self->extra_envs[i])) == NULL) { | ||
| errlogSevPrintf(errlogMinor, "strdup error for copying %s to new_extra_envs[%zu] from addReccasterEnvVars\n", self->extra_envs[i], i); | ||
| ret = 1; | ||
| break; | ||
| } | ||
| } | ||
| int found_dup; | ||
| /* sanitize input - check for dups and empty args */ | ||
| if(!ret) { | ||
| for(i=0; i < argc; i++) { | ||
| if(argv[i] == NULL) { | ||
| errlogSevPrintf(errlogMinor, "Arg is NULL for addReccasterEnvVars\n"); | ||
| continue; | ||
| } | ||
| else if(argv[i][0] == '\0') { | ||
| errlogSevPrintf(errlogMinor, "Arg is empty for addReccasterEnvVars\n"); | ||
| continue; | ||
| } | ||
| found_dup = 0; | ||
| /* check if dup in self->default_envs */ | ||
| for(j = 0; default_envs[j]; j++) { | ||
| if(strcmp(argv[i], default_envs[j]) == 0) { | ||
| found_dup = 1; | ||
| errlogSevPrintf(errlogMinor, "Env var %s is already in env list sent by reccaster by default\n", argv[i]); | ||
| break; | ||
| } | ||
| } | ||
| if(found_dup) { | ||
| continue; | ||
| } | ||
| /* check if dup in self->extra_envs */ | ||
| for(j = 0; j < num_new_extra_envs; j++) { | ||
| if(new_extra_envs[j] == NULL) { | ||
| continue; | ||
| } | ||
| if(strcmp(argv[i], new_extra_envs[j]) == 0) { | ||
| found_dup = 1; | ||
| errlogSevPrintf(errlogMinor, "Env var %s is already in extra_envs list\n", argv[i]); | ||
| break; | ||
| } | ||
| } | ||
| if(found_dup) { | ||
| continue; | ||
| for(i=0; i < argc; i++) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lots of duplication in this loop and in the one for exclude patterns, just a thought... |
||
| if(argv[i][0] == '\0') { | ||
| errlogSevPrintf(errlogMinor, "Arg is empty for addReccasterEnvVars\n"); | ||
| continue; | ||
| } | ||
| found_dup = 0; | ||
| /* check if dup in self->default_envs */ | ||
| for(j = 0; default_envs[j]; j++) { | ||
| if(strcmp(argv[i], default_envs[j]) == 0) { | ||
| found_dup = 1; | ||
| break; | ||
| } | ||
| if((new_extra_envs[num_new_extra_envs] = strdup(argv[i])) == NULL) { | ||
| errlogSevPrintf(errlogMinor, "strdup error for copying %s to new_extra_envs[%d] from addReccasterEnvVars\n", argv[i], num_new_extra_envs); | ||
| ret = 1; | ||
| } | ||
| if(found_dup) { | ||
| errlogSevPrintf(errlogMinor, "Env var %s is already in env list sent by reccaster by default\n", argv[i]); | ||
| continue; | ||
| } | ||
| /* check if dup in self->extra_envs */ | ||
| ELLNODE *cur = ellFirst(&self->extra_envs); | ||
| while (cur != NULL) { | ||
| string_list_t *temp = (string_list_t *)cur; | ||
| if (strcmp(argv[i], temp->item_str) == 0) { | ||
| found_dup = 1; | ||
| break; | ||
| } | ||
| /* this is a valid arg and we have added the new env var to our array, increment new_extra_envs count */ | ||
| num_new_extra_envs++; | ||
| cur = ellNext(cur); | ||
| } | ||
| if(found_dup) { | ||
| errlogSevPrintf(errlogMinor, "Env var %s is already in extra_envs list\n", argv[i]); | ||
| continue; | ||
| } | ||
| string_list_t *new_list = malloc(sizeof(string_list_t)); | ||
| if (new_list == NULL) { | ||
| errlogSevPrintf(errlogMajor, "Error in addReccasterEnvVars - malloc error for creating linked list node"); | ||
| break; | ||
| } | ||
| new_list->item_str = strdup(argv[i]); | ||
|
Comment on lines
+131
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fyi. These two allocations could be combined into one. For example: https://github.com/epics-base/epics-base/blob/e4fbd83c93a68a1d9cb863ff341735388d246347/modules/libcom/src/iocsh/afterIocRunning.c#L62-L65 const size_t cmd_len = strlen(cmd) + 1;
struct cmditem *const item = mallocMustSucceed(sizeof(struct cmditem) + cmd_len, "afterIocRunning");
item->cmd = (char *)(item + 1);
memcpy(item->cmd, cmd, cmd_len);Also, iteration could be done in a more compact fashion: https://github.com/epics-base/epics-base/blob/e4fbd83c93a68a1d9cb863ff341735388d246347/modules/libcom/src/iocsh/initHooks.c#L64-L65 for(cur = ellFirst(&functionList); cur; cur = ellNext(cur)) {
const initHookLink *fn = CONTAINER(cur, initHookLink, node); |
||
| if (new_list->item_str == NULL) { | ||
| errlogSevPrintf(errlogMajor, "Error in addReccasterEnvVars - strdup error for copying %s to new->item_str from addReccasterEnvVars\n", argv[i]); | ||
| free(new_list); /* frees if strdup fails */ | ||
| break; | ||
| } | ||
| ellAdd(&self->extra_envs, &new_list->node); | ||
| } | ||
| /* if we have no allocation issues and have at least one new env var that is valid, add to self->extra_envs */ | ||
| if(!ret && num_new_extra_envs > self->num_extra_envs) { | ||
| /* from this point, nothing can fail */ | ||
| char ** tmp; | ||
| tmp = self->extra_envs; /* swap pointers so we can clean up new_extra_envs on success/failure */ | ||
| self->extra_envs = new_extra_envs; | ||
| new_extra_envs = tmp; | ||
|
|
||
| new_extra_envs_size = self->num_extra_envs; /* with swap of pointers also swap size */ | ||
| self->num_extra_envs = num_new_extra_envs; | ||
| } | ||
| /* cleanup new_extra_envs[] on success or failure */ | ||
| for(i = 0; i < new_extra_envs_size; i++) { | ||
| free(new_extra_envs[i]); | ||
| } | ||
| free(new_extra_envs); | ||
| epicsMutexUnlock(self->lock); | ||
|
|
||
| if(ret) { | ||
| errlogSevPrintf(errlogMajor, "Error in addReccasterEnvVars - reccaster might not send the extra env vars specified\n"); | ||
| } | ||
| } | ||
|
|
||
| static const iocshArg addReccasterEnvVarsArg0 = { "environmentVar", iocshArgArgv }; | ||
|
|
@@ -200,6 +162,74 @@ static void addReccasterEnvVarsCallFunc(const iocshArgBuf *args) | |
| addReccasterEnvVars(&thecaster, args[0].aval.ac, args[0].aval.av); | ||
| } | ||
|
|
||
| void addReccasterExcludePattern(caster_t* self, int argc, char **argv) { | ||
| size_t i; | ||
| argv++; argc--; /* skip function arg */ | ||
| if (argc < 1) { | ||
| errlogSevPrintf(errlogMinor, "At least one argument expected for addReccasterExcludePattern\n"); | ||
| return; | ||
| } | ||
| epicsMutexMustLock(self->lock); | ||
| if (self->shutdown) { | ||
| /* shutdown in progress, silent no-op */ | ||
| epicsMutexUnlock(self->lock); | ||
| return; | ||
| } | ||
| /* error if called after iocInit() */ | ||
| if (self->current != casterStateInit) { | ||
| errlogSevPrintf(errlogMinor, "addReccasterExcludePattern called after iocInit() when reccaster might already be connected. Not supported\n"); | ||
| epicsMutexUnlock(self->lock); | ||
| return; | ||
| } | ||
|
|
||
| for (i = 0; i < argc; i++) { | ||
| if (argv[i][0] == '\0') { | ||
| errlogSevPrintf(errlogMinor, "Arg is empty for addReccasterExcludePattern\n"); | ||
| continue; | ||
| } | ||
| /* check duplicates */ | ||
| int dup = 0; | ||
| ELLNODE *cur = ellFirst(&self->exclude_patterns); | ||
| while (cur != NULL) { | ||
| string_list_t *temp = (string_list_t *)cur; | ||
| if (strcmp(argv[i], temp->item_str) == 0) { | ||
| dup = 1; | ||
| break; | ||
| } | ||
| cur = ellNext(cur); | ||
| } | ||
| if (dup) { | ||
| errlogSevPrintf(errlogMinor, "Duplicate pattern %s in addReccasterExcludePattern\n", argv[i]); | ||
| continue; | ||
| } | ||
| string_list_t *new_list = malloc(sizeof(string_list_t)); | ||
| if (new_list == NULL) { | ||
| errlogSevPrintf(errlogMajor, "Error in addReccasterExcludePattern - malloc error for creating linked list node"); | ||
| break; | ||
| } | ||
| new_list->item_str = strdup(argv[i]); | ||
| if (new_list->item_str == NULL) { | ||
| errlogSevPrintf(errlogMajor, "Error in addReccasterExcludePattern - strdup error for copying %s to new->item_str from addReccasterExcludePattern\n", argv[i]); | ||
| free(new_list); /* frees if strdup fails */ | ||
| break; | ||
| } | ||
| ellAdd(&self->exclude_patterns, &new_list->node); | ||
| } | ||
| epicsMutexUnlock(self->lock); | ||
| } | ||
|
|
||
| static const iocshArg addReccasterExcludePatternArg0 = { "excludePattern", iocshArgArgv }; | ||
| static const iocshArg * const addReccasterExcludePatternArgs[] = { &addReccasterExcludePatternArg0 }; | ||
| static const iocshFuncDef addReccasterExcludePatternFuncDef = { | ||
| "addReccasterExcludePattern", | ||
| 1, | ||
| addReccasterExcludePatternArgs | ||
| }; | ||
|
|
||
| static void addReccasterExcludePatternCallFunc(const iocshArgBuf *args) { | ||
| addReccasterExcludePattern(&thecaster, args[0].aval.ac, args[0].aval.av); | ||
| } | ||
|
|
||
| static void reccasterRegistrar(void) | ||
| { | ||
| osiSockAttach(); | ||
|
|
@@ -210,6 +240,7 @@ static void reccasterRegistrar(void) | |
| thepriv.laststate=casterStateInit; | ||
| strcpy(thepriv.lastmsg, "Initializing"); | ||
| iocshRegister(&addReccasterEnvVarsFuncDef,addReccasterEnvVarsCallFunc); | ||
| iocshRegister(&addReccasterExcludePatternFuncDef,addReccasterExcludePatternCallFunc); | ||
| } | ||
|
|
||
| static long init_record(void* prec) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,12 +71,15 @@ static int pushEnv(caster_t *caster) | |
| } | ||
|
|
||
| epicsMutexMustLock(caster->lock); | ||
| for (i = 0; !ret && i < caster->num_extra_envs; i++) { | ||
| const char *val = getenv(caster->extra_envs[i]); | ||
| ELLNODE *env = ellFirst(&caster->extra_envs); | ||
| while (!ret && env != NULL) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that this change has addressed my previous comment about the loop iteration. Consider squashing these two commits? |
||
| string_list_t *temp = (string_list_t *)env; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love the variable name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. string_list_t *envptr = CONTAINER(env, string_list_t, node);
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... Corrected missing first argument |
||
| const char *val = getenv(temp->item_str); | ||
| if (val && val[0] != '\0') | ||
| ret = casterSendInfo(caster, 0, caster->extra_envs[i], val); | ||
| ret = casterSendInfo(caster, 0, temp->item_str, val); | ||
| if (ret) | ||
| casterMsg(caster, "Error sending env %s", caster->extra_envs[i]); | ||
| casterMsg(caster, "Error sending env %s", temp->item_str); | ||
| env = ellNext(env); | ||
| } | ||
| epicsMutexUnlock(caster->lock); | ||
|
|
||
|
|
@@ -87,12 +90,21 @@ static int pushRecord(caster_t *caster, DBENTRY *pent) | |
| { | ||
| dbCommon *prec = pent->precnode->precord; | ||
| ssize_t rid; | ||
| ELLNODE *cur; | ||
| int ret = 0; | ||
| long status; | ||
|
|
||
| if(dbIsAlias(pent)) | ||
| return 0; | ||
|
|
||
| cur = ellFirst(&caster->exclude_patterns); | ||
| while (cur != NULL) { | ||
| string_list_t *temp = (string_list_t *)cur; | ||
| if(epicsStrGlobMatch(prec->name, temp->item_str)) | ||
| return 0; | ||
| cur = ellNext(cur); | ||
| } | ||
|
|
||
| rid = casterSendRecord(caster, prec->rdes->name, prec->name); | ||
| if(rid<=0) | ||
| return rid; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this lock/unlock?