Changeset View
Changeset View
Standalone View
Standalone View
sbin/ifconfig/ifconfig.c
| Show First 20 Lines • Show All 1,404 Lines • ▼ Show 20 Lines | if ((descr = reallocf(descr, descrlen)) != NULL) { | ||||
| descr); | descr); | ||||
| } else if (ifr.ifr_buffer.length > descrlen) { | } else if (ifr.ifr_buffer.length > descrlen) { | ||||
| descrlen = ifr.ifr_buffer.length; | descrlen = ifr.ifr_buffer.length; | ||||
| continue; | continue; | ||||
| } | } | ||||
| } | } | ||||
| } else | } else | ||||
| warn("unable to allocate memory for interface" | warn("unable to allocate memory for interface" | ||||
| "description"); | " description"); | ||||
| break; | break; | ||||
| } | } | ||||
| for (;;) { | |||||
| if ((descr = reallocf(descr, descrlen)) != NULL) { | |||||
kp: I'd reverse the logic here. If the allocation fails break out. That moves the bulk of the code… | |||||
| ifr.ifr_buffer.buffer = descr; | |||||
| ifr.ifr_buffer.length = descrlen; | |||||
| if (ioctl(s, SIOCGIFDNAME, &ifr) == 0) { | |||||
| if (ifr.ifr_buffer.buffer == descr) { | |||||
| if (strlen(descr) > 0 && | |||||
| strcmp(name, descr) != 0) { | |||||
Done Inline ActionsIs there a point in doing repeated allocations here? We know the maximum length of an interface name. kp: Is there a point in doing repeated allocations here? We know the maximum length of an interface… | |||||
| printf("\tinitname: %s\n", | |||||
| descr); | |||||
| } | |||||
| } else if (ifr.ifr_buffer.length > descrlen) { | |||||
| descrlen = ifr.ifr_buffer.length; | |||||
| continue; | |||||
| } | |||||
| } | |||||
| } else | |||||
| warn("unable to allocate memory for interface" | |||||
| " initname"); | |||||
Not Done Inline ActionsThat's unrelated white space; just commit that ;-) Usually we add the space to the end of line but I assume it's at width=80 already? bz: That's unrelated white space; just commit that ;-) Usually we add the space to the end of… | |||||
| break; | |||||
| } | |||||
| if (ioctl(s, SIOCGIFCAP, (caddr_t)&ifr) == 0) { | if (ioctl(s, SIOCGIFCAP, (caddr_t)&ifr) == 0) { | ||||
Done Inline ActionsThat looks wrong. If allocation fails we do the ioctl(), otherwise we warn about an allocation failure. kp: That looks wrong. If allocation fails we do the ioctl(), otherwise we warn about an allocation… | |||||
Done Inline ActionsGood catch, while refactoring I had looked at trying to just quit early if allocation failed, to reduce another level of indent, but it didn't work out, and this didn't get switched back. allanjude: Good catch, while refactoring I had looked at trying to just quit early if allocation failed… | |||||
| if (ifr.ifr_curcap != 0) { | if (ifr.ifr_curcap != 0) { | ||||
| printb("\toptions", ifr.ifr_curcap, IFCAPBITS); | printb("\toptions", ifr.ifr_curcap, IFCAPBITS); | ||||
| putchar('\n'); | putchar('\n'); | ||||
| } | } | ||||
| if (supmedia && ifr.ifr_reqcap != 0) { | if (supmedia && ifr.ifr_reqcap != 0) { | ||||
| printb("\tcapabilities", ifr.ifr_reqcap, IFCAPBITS); | printb("\tcapabilities", ifr.ifr_reqcap, IFCAPBITS); | ||||
| putchar('\n'); | putchar('\n'); | ||||
| } | } | ||||
| ▲ Show 20 Lines • Show All 294 Lines • Show Last 20 Lines | |||||
I'd reverse the logic here. If the allocation fails break out. That moves the bulk of the code one level of indentation down.