[Crash-utility] [PATCH] Add -C option for search

[Dave Anderson]

----- Original Message -----
> Hello Dave,
> 
> I add a new option -C for sub-command search to display memory contents
> just before and after the search target.  The number of the memory unit
> displayed is specified by -C.
> 
> for example,
> crash> search -p -C 3 dddd
> 17b5100: dddddddddddddddd dddddddddddddddd dddddddddddddddd
> 17b5118: dddd cdcdcdcdcdcdcdcd cdcdcdcdcdcdcdcd
> 17b5130: cdcdcdcdcdcdcdcd
> --
> 85d04f818: 29 ffff88085d04f818 ffffea001d3f7e30
> 85d04f830: dddd 1000 ffff88085b48a000
> 85d04f848: ffff880876383b80
> --
> 8752cfec8: ddd9 dddb dddb
> 8752cfee0: dddd dddd dddf
> 8752cfef8: dddf
> --
> 8752cfed0: dddb dddb dddd
> 8752cfee8: dddd dddf dddf
> 8752cff00: dde1
> --
> crash>
> 
> 
> Thanks.
> Zhang Yanfei

Hello Zhang,

I'm going to NAK this patch for a few reasons.

First, for an option that will very rarely be used, it adds an
*enormous* amount of code -- the patch is almost 1000 lines long!
I mean you've almost re-written the whole memory-search subsystem!
And the code is not particularly easy to understand, and quite
frankly, I do not want to get stuck maintaining it.

Secondly, on a live system, even if the option is *not* used,
a simple "search -k" is 30% slower.  I'm not sure why it's
so slow, although interestingly enough, when run on a dumpfile, 
it's basically the same speed.  It's weird -- you would expect 
it to be faster on a live system...

Third, it can be done in a much simpler manner, and done without
affecting the "normal" course of events.  

Basically you've got the six functions that need to be modified 
to display the context memory before and after a "found" piece 
of data, i.e., search_ulong(), search_ulong_p(), search_uint(),
search_uint_p(), search_ushort() and search_ushort_p().

What I suggest is that you do something like this in each of 
the 6 relevant functions:
  
  static ulong
  search_ulong(ulong *bufptr, ulong addr, int longcnt, struct searchinfo *si)
  {
          int i, j;
+         ulong *bufstart = bufptr;
+	  ulong *bufend = bufptr + longcnt;
          ulong mask = si->s_parms.s_ulong.mask;
          for (i = 0; i < longcnt; i++, bufptr++, addr += sizeof(long)) {
                  for (j = 0; j < si->vcnt; j++) {
-                         if (SEARCHMASK(*bufptr) == SEARCHMASK(si->s_parms.s_ulong.value[j]))
-                                 fprintf(fp, "%lx: %lx\n", addr, *bufptr);
+                         if (SEARCHMASK(*bufptr) == SEARCHMASK(si->s_parms.s_ulong.value[j])) {
+                                 if (si->c_num)
+                                         display_with_pre_and_post(si, bufstart, bufend, addr, *bufptr);
+                                 else
+                                         fprintf(fp, "%lx: %lx\n", addr, *bufptr);
+                 }
          }
          return addr;
  }
      
and then put all of your functionality in the display_with_pre_and_post() 
function.  In most cases, the data before and after the target data 
will be located in the pre-read buffer.  In cases where the extra data 
is outside the pre-read buffer, then you can simply do an extra readmem()
to satisfy the request.  

Please -- just keep it simple!

Thanks,
   Dave