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

----- Original Message ----- 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