This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Patch for unattended setup (updated)


"Dr. Frank Lee" wrote:

> Apologies, _this_ is the new patch.

> -
> -    theLog->exit (0);
> +    if (rebootneeded) {
> +      theLog->exit (IDS_REBOOT_REQUIRED);
> +    } else {
> +      theLog->exit (0);
> +    }
>    }

The GNU style has a brace on its own line.

> +  if (unattended_mode) {

Likewise.

> +  for_each(packages.begin(), packages.end(), visit_if(mem_fun(&packagemeta::addToCategoryBase), mem_fun(&packagemeta::isManuallyWanted)));

I'm not sure this is very performant.  'isManuallyWanted' is a rather
expensive function and it seems like a waste to have to perform all of
this:

> +static StringOption PackageOption ("", 's', "software", "Software packages to include");
> +
> +bool packagemeta::isManuallyWanted() const
> +{
> +  string packages_option = PackageOption;
> +  string tname;
> +  /* Split the packages listed in the option up */
> +  string::size_type loc = packages_option.find(",",0);
> +  bool bReturn = false;
> +  while ( loc != string::npos ) {
> +    tname = packages_option.substr(0,loc);
> +    packages_option = packages_option.substr(loc+1);
> +    bReturn = bReturn || (name.compare(tname) == 0);
> +    loc = packages_option.find(",",0);
> +  }
> +  /* At this point, no "," exists in packages_option */
> +  bReturn = bReturn || (name.compare(packages_option) == 0);
> +  return bReturn;
> +}

...once for every instance of every package, which is hundreds. 
Couldn't the string parsing stuff be done just once to convert it to a
vector or list, so that it doesn't have to be reparsed over and over?

Brian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]