Thanks Alex. I'm out of town during the weekend. Here's the refined patch. I addressed all the issues you pointed out.
BTW, how to identify these TAB and WHITESPACE issues by search the patch? (we cannot search WHITESPACE, right? :P) Thanks a lot! Tianyin On Fri, Jan 11, 2013 at 4:18 PM, Alex Rousskov <[email protected]> wrote: > On 01/11/2013 01:06 AM, Tianyin Xu wrote: >> Hi, Amos, Alex, >> >> Thanks a lot for your comments! They are really good advice. >> >> The updated patch is attached. It addresses all the issues you >> mentioned except whether to deprecate the "enable"/"disable" options, >> which is worth of discussion. >> >> Thanks, >> Tianyin >> >> >> On Thu, Jan 10, 2013 at 8:14 AM, Alex Rousskov >> <[email protected]> wrote: >>> On 01/05/2013 05:12 PM, Tianyin Xu wrote: >>> >>>> @@ -193,6 +290,8 @@ >>>> return false; >>>> } else if ((port = strtol(token, &tmp, 10)), !*tmp) { >>>> /* port */ >>>> + port = xatos(token); >>>> + >>>> } else { >>>> host = token; >>>> port = 0; >>> >>> Please do not set "port" twice because it is confusing and the comma >>> operator in the expression only makes things worse. Do something like >>> this instead: >>> >>> else if (strtol(token, &tmp, 10) && !*tmp) { >>> port = xatos(token); >>> } > > > >> + } else if (strtol(token, &tmp, 10), !*tmp) { >> /* port */ >> + port = xatos(token); >> + >> } else { > > Please replace the comma operator with the "&&" operator. > > Please remove the empty line added in the patch. > > Please remove the /* port */ comment as no longer needed. > > >> - } else if ((port = strtol(token, &junk, 10)), !*junk) { >> + } else if (strtol(token, &junk, 10), !*junk) { >> /* port */ >> + port = xatos(token); > > Same comments apply here, except there is no extra empty line to remove. > > >> while (port && i < WCCP2_NUMPORTS) { >> - p = strtol(port, &end, 0); >> + p = xatoi(port);TABHERE >> > >> +unsigned int >> +xatoui(const char *token) >> +{ >> + int64_t input = xatoll(token, 10); >> + if (input < 0) { >> +TABHEREdebugs(0, DBG_PARSE_NOTE(DBG_IMPORTANT), "ERROR: The input value '" >> << token << "' cannot be less than 0."); >> + self_destruct(); >> + } > > Please replace the leading tab with spaces and remove trailing > whitespace. BTW, such things are easy to find by searching the patch. > > The above polishing touches can be done during commit IMO if you do not > want to resubmit. > > > Thanks a lot, > > Alex. > -- Tianyin XU, http://cseweb.ucsd.edu/~tixu/
config.patch
Description: Binary data
