Changeset View
Changeset View
Standalone View
Standalone View
usr.bin/tset/map.c
Show All 12 Lines | |||||
*/ | */ | ||||
void | void | ||||
add_mapping(const char *port, char *arg) | add_mapping(const char *port, char *arg) | ||||
{ | { | ||||
MAP *mapp; | MAP *mapp; | ||||
char *copy, *p, *termp; | char *copy, *p, *termp; | ||||
copy = strdup(arg); | copy = strdup(arg); | ||||
if (copy == NULL) | |||||
errx(1, "malloc"); | |||||
mapp = malloc(sizeof(MAP)); | mapp = malloc(sizeof(MAP)); | ||||
if (copy == NULL || mapp == NULL) | if (mapp == NULL) { | ||||
free(copy); | |||||
errx(1, "malloc"); | errx(1, "malloc"); | ||||
} | |||||
ed: I don't understand this change. What's the advantage of calling `free()` if you're already… | |||||
mapp->next = NULL; | mapp->next = NULL; | ||||
if (maplist == NULL) | if (maplist == NULL) | ||||
cur = maplist = mapp; | cur = maplist = mapp; | ||||
else { | else { | ||||
cur->next = mapp; | cur->next = mapp; | ||||
cur = mapp; | cur = mapp; | ||||
} | } | ||||
Show All 24 Lines | |||||
*termp = '\0'; | *termp = '\0'; | ||||
/* If a NOT conditional, reverse the test. */ | /* If a NOT conditional, reverse the test. */ | ||||
if (mapp->conditional & NOT) | if (mapp->conditional & NOT) | ||||
mapp->conditional = ~mapp->conditional & (EQ | GT | LT); | mapp->conditional = ~mapp->conditional & (EQ | GT | LT); | ||||
/* If user specified a port with an option flag, set it. */ | /* If user specified a port with an option flag, set it. */ | ||||
done: if (port) { | done: if (port) { | ||||
/* XXX no way to free 'copy' */ | |||||
if (mapp->porttype) | if (mapp->porttype) | ||||
Not Done Inline ActionsI think assignments like these are typically not done in our codebase, especially not for local variables. Statements like these tend to have no effect, as the compiler will optimize it out anyway. Apart from that, it looks good! ed: I think assignments like these are typically not done in our codebase, especially not for local… | |||||
badmopt: errx(1, "illegal -m option format: %s", copy); | badmopt: errx(1, "illegal -m option format: %s", copy); | ||||
mapp->porttype = strdup(port); | mapp->porttype = strdup(port); | ||||
} | } | ||||
#ifdef MAPDEBUG | #ifdef MAPDEBUG | ||||
(void)printf("port: %s\n", mapp->porttype ? mapp->porttype : "ANY"); | (void)printf("port: %s\n", mapp->porttype ? mapp->porttype : "ANY"); | ||||
(void)printf("type: %s\n", mapp->type); | (void)printf("type: %s\n", mapp->type); | ||||
(void)printf("conditional: "); | (void)printf("conditional: "); | ||||
p = ""; | p = ""; | ||||
if (mapp->conditional & GT) { | if (mapp->conditional & GT) { | ||||
(void)printf("GT"); | (void)printf("GT"); | ||||
p = "/"; | p = "/"; | ||||
} | } | ||||
if (mapp->conditional & EQ) { | if (mapp->conditional & EQ) { | ||||
(void)printf("%sEQ", p); | (void)printf("%sEQ", p); | ||||
p = "/"; | p = "/"; | ||||
} | } | ||||
Not Done Inline ActionsCould you please move the call to free() closer to to the site where the variable is last used? Thanks! ed: Could you please move the call to `free()` closer to to the site where the variable is last… | |||||
if (mapp->conditional & LT) | if (mapp->conditional & LT) | ||||
(void)printf("%sLT", p); | (void)printf("%sLT", p); | ||||
(void)printf("\nspeed: %d\n", mapp->speed); | (void)printf("\nspeed: %d\n", mapp->speed); | ||||
#endif | #endif | ||||
free(copy); | |||||
} | } | ||||
/* | /* | ||||
* Return the type of terminal to use for a port of type 'type', as specified | * Return the type of terminal to use for a port of type 'type', as specified | ||||
* by the first applicable mapping in 'map'. If no mappings apply, return | * by the first applicable mapping in 'map'. If no mappings apply, return | ||||
* 'type'. | * 'type'. | ||||
*/ | */ | ||||
const char * | const char * | ||||
Show All 12 Lines |
I don't understand this change. What's the advantage of calling free() if you're already going to call errx()?