Re: [PATCH 1/2] Make verbose give a reason why a rebase is needed

[Corinna Vinschen]

On Feb  9 11:59, Jon Turney wrote:
> ---
>  rebase.c | 47 ---
>  1 file changed, 40 insertions(+), 7 deletions(-)
> 
> diff --git a/rebase.c b/rebase.c
> index 6f98d37..0aad1b2 100644
> --- a/rebase.c
> +++ b/rebase.c
> @@ -649,7 +649,16 @@ merge_image_info ()
>   {
> /* Reuse the old address if possible. */
> if (match->slot_size < img_info_list[i].slot_size)
> - match->base = 0;
> + {
> +   if (verbose)
> + fprintf (stderr, "rebasing %s because it won't fit in 
> it's old slot size\n", img_info_list[i].name);
> +   match->base = 0;
> + }
> +   else
> + {
> +   if (verbose)

This would ideally be an 

  else if (verbose)
fprintf ();

> - img_info_list[i].base = 0;
> + {
> +   /* Not in database yet.  Set base to 0 to choose a new one. */
> +   img_info_list[i].base = 0;
> +   if (verbose)
> + fprintf (stderr, "rebasing %s because not in database yet\n", 
> img_info_list[i].name);

In the "reuse old address" case you have

  if (verbose)
printf
  set var = 0

here and in later cases you have

  set var = 0;
  if (verbose)
printf

I'd prefer to have these in the same order.

With this minor tweak patch is ok.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat


signature.asc
Description: PGP signature

[PATCH 1/2] Make verbose give a reason why a rebase is needed

[Jon Turney]

---
 rebase.c | 47 ---
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/rebase.c b/rebase.c
index 6f98d37..0aad1b2 100644
--- a/rebase.c
+++ b/rebase.c
@@ -649,7 +649,16 @@ merge_image_info ()
{
  /* Reuse the old address if possible. */
  if (match->slot_size < img_info_list[i].slot_size)
-   match->base = 0;
+   {
+ if (verbose)
+   fprintf (stderr, "rebasing %s because it won't fit in 
it's old slot size\n", img_info_list[i].name);
+ match->base = 0;
+   }
+ else
+   {
+ if (verbose)
+   fprintf (stderr, "rebasing %s because it's not located 
at it's old slot\n", img_info_list[i].name);
+   }
  match->flag.needs_rebasing = 1;
}
  /* Unconditionally overwrite old with new size. */
@@ -668,8 +677,12 @@ merge_image_info ()
  img_info_list[i--] = img_info_list[--img_info_size];
}
  else if (!img_info_list[i].flag.cannot_rebase)
-   /* Not in database yet.  Set base to 0 to choose a new one. */
-   img_info_list[i].base = 0;
+   {
+ /* Not in database yet.  Set base to 0 to choose a new one. */
+ img_info_list[i].base = 0;
+ if (verbose)
+   fprintf (stderr, "rebasing %s because not in database yet\n", 
img_info_list[i].name);
+   }
}
 }
   if (!img_info_rebase_start || force_rebase_flag)
@@ -682,7 +695,11 @@ merge_image_info ()
  if (i < img_info_rebase_start)
set_cannot_rebase (&img_info_list[i]);
  if (!img_info_list[i].flag.cannot_rebase)
-   img_info_list[i].base = 0;
+   {
+ img_info_list[i].base = 0;
+ if (verbose)
+   fprintf (stderr, "rebasing %s because forced or database 
missing\n", img_info_list[i].name);
+   }
}
   img_info_rebase_start = 0;
 }
@@ -725,6 +742,8 @@ merge_image_info ()
 in the first place. */
  if (cur_base != img_info_list[i].base)
{
+ if (verbose)
+   fprintf (stderr, "rebasing %s because it's base has changed 
(due to being reinstalled?)\n", img_info_list[i].name);
  img_info_list[i].flag.needs_rebasing = 1;
  /* Set cur_base to the old base to simplify subsequent tests. */
  cur_base = img_info_list[i].base;
@@ -733,17 +752,29 @@ merge_image_info ()
 anymore, rebase this DLL from scratch. */
  if (i + 1 < img_info_rebase_start
  && cur_base + slot_size + offset >= img_info_list[i + 1].base)
-   img_info_list[i].base = 0;
+   {
+ img_info_list[i].base = 0;
+ if (verbose)
+   fprintf (stderr, "rebasing %s because it won't fit in it's old 
slot without overlapping next DLL\n", img_info_list[i].name);
+   }
  /* Does the previous DLL reach into the address space of this
 DLL?  This happens if the previous DLL is not rebaseable. */
  else if (i > 0 && cur_base < img_info_list[i - 1].base
   + img_info_list[i + 1].slot_size)
-   img_info_list[i].base = 0;
+   {
+ img_info_list[i].base = 0;
+ if (verbose)
+   fprintf (stderr, "rebasing %s because previous DLL now 
overlaps\n", img_info_list[i].name);
+   }
  /* Does the file match the base address requirements?  If not,
 rebase from scratch. */
  else if ((down_flag && cur_base + slot_size + offset >= image_base)
   || (!down_flag && cur_base < image_base))
-   img_info_list[i].base = 0;
+   {
+ img_info_list[i].base = 0;
+ if (verbose)
+   fprintf (stderr, "rebasing %s because it's base address is 
outside the expected area\n", img_info_list[i].name);
+   }
}
   /* Unconditionally overwrite old with new size. */
   img_info_list[i].size = cur_size;
@@ -900,6 +931,8 @@ collect_image_info (const char *pathname)
   img_info_list[img_info_size].slot_size
 = roundup2 (img_info_list[img_info_size].size, ALLOCATION_SLOT);
   img_info_list[img_info_size].flag.needs_rebasing = 1;
+  if (verbose)
+fprintf (stderr, "rebasing %s because filename given on command line\n", 
img_info_list[img_info_size].name);
   img_info_list[img_info_size].flag.cannot_rebase = 0;
   /* This back and forth from POSIX to Win32 is a way to get a full path
  more thoroughly.  For instance, the difference between /bin and
-- 
2.16.1