Messages in this thread | | | Date | Sun, 10 Feb 2008 13:46:45 +0100 | From | Marcin Slusarz <> | Subject | Re: [3/6] kgdb: core |
| |
On Sun, Feb 10, 2008 at 08:13:31AM +0100, Ingo Molnar wrote: > + } else { > + while (count-- > 0) { > + unsigned char ch; > + > + if (probe_kernel_address(mem, ch)) { > + kgdb_may_fault = 0; > + return ERR_PTR(-EINVAL); > + } > + mem++; > + *buf++ = hexchars[ch >> 4]; > + *buf++ = hexchars[ch & 0xf]; use pack_hex_byte?
> +/* > + * While we find nice hex chars, build a long_val. > + * Return number of chars processed. > + */ > +int kgdb_hex2long(char **ptr, long *long_val) > +{ > + int hex_val; > + int num = 0; > + > + *long_val = 0; > + > + while (**ptr) { > + hex_val = hex(**ptr); > + if (hex_val >= 0) { > + *long_val = (*long_val << 4) | hex_val; > + num++; > + } else > + break; > + > + (*ptr)++; > + } if (hex_val < 0) break; *long_val = (*long_val << 4) | hex_val; num++; (*ptr)++;
> +/* > + * SW breakpoint management: > + */ > +static int kgdb_activate_sw_breakpoints(void) > +{ > + unsigned long addr; > + int error = 0; > + int i; > + > + for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) { > + if (kgdb_break[i].state != BP_SET) > + continue; > + > + addr = kgdb_break[i].bpt_addr; > + error = kgdb_arch_set_breakpoint(addr, > + kgdb_break[i].saved_instr); > + if (error) > + return error; > + > + if (CACHE_FLUSH_IS_SAFE) { > + if (current->mm && addr < TASK_SIZE) { > + flush_cache_range(current->mm->mmap_cache, > + addr, addr + BREAK_INSTR_SIZE); > + } else { > + flush_icache_range(addr, addr + > + BREAK_INSTR_SIZE); > + } > + } unneeded braces (here and in many other places)
> +/* Handle the '?' status packets */ > +static void gdb_cmd_status(struct kgdb_state *ks) > +{ > + /* > + * We know that this packet is only sent > + * during initial connect. So to be safe, > + * we clear out our breakpoints now in case > + * GDB is reconnecting. > + */ > + remove_all_break(); > + > + /* > + * Also, if we haven't been able to roundup all > + * CPUs, send an 'O' packet informing the user > + * as much. Only need to do this once. > + */ > + if (!ks->all_cpus_synced) > + kgdb_msg_write("Not all CPUs have been synced for KGDB\n", 39); > + > + remcom_out_buffer[0] = 'S'; > + remcom_out_buffer[1] = hexchars[ks->signo >> 4]; > + remcom_out_buffer[2] = hexchars[ks->signo % 16]; use pack_hex_byte or & 0xf
> + if (ks->threadid < pid_max) { > + kgdb_mem2hex(getthread(ks->linux_regs, > + ks->threadid)->comm, > + remcom_out_buffer, 16); > + } else { > + if (ks->threadid >= pid_max + num_online_cpus()) { > + kgdb_shadowinfo(ks->linux_regs, > + remcom_out_buffer, > + ks->threadid - pid_max - > + num_online_cpus()); > + } else { > + static char tmpstr[23 + BUF_THREAD_ID_SIZE]; > + sprintf(tmpstr, "Shadow task %d for pid 0", > + (int)(ks->threadid - pid_max)); > + kgdb_mem2hex(tmpstr, remcom_out_buffer, > + strlen(tmpstr)); > + } > + } if () else if () else
will look better
> + if (*(ptr++) != ',') { > + error_packet(remcom_out_buffer, -EINVAL); > + return; > + } else { no else needed
> + if (kgdb_hex2long(&ptr, &addr)) { > + if (*(ptr++) != ',' || > + !kgdb_hex2long(&ptr, &length)) { > + error_packet(remcom_out_buffer, -EINVAL); > + return; > + } > + } else { > + error_packet(remcom_out_buffer, -EINVAL); > + return; > + } > + } if (!kgdb_hex2long()) { error_packet(); return; }
if (*(ptr++) (...)) (...)
Marcin
| |