On 11/11/2015 03:52 PM, Gabriel Krisman Bertazi wrote:
> Heitor Ricardo Alves de Siqueira <hal...@linux.vnet.ibm.com> writes:
>
>> +static void dump_entry_data(struct ipr_dump_entry_header *hdr)
>> +{
>> + u32 i;
>> + char *s;
>> +
>> + switch (hdr->id) {
>> + case ID_IOA_DUMP:
>> + fprintf(out_fp, "Smart Dump Table Data:\n");
>> + print_sdt(hdr->offset);
>> + break;
>> + case ID_IOA_LOC:
>> + fprintf(out_fp, "IOA Location:\n");
>> + s = dump_map+hdr->offset;
>> + fprintf(out_fp, "\t %.*s\n\n", hdr->len, s);
>> + break;
>> + case ID_DRV_VER:
>> + fprintf(out_fp, "Driver Version:\n");
>> + s = dump_map+hdr->offset;
>> + fprintf(out_fp, "\t %.*s\n\n", hdr->len, s);
>> + break;
>> + case ID_DRV_TRACE:
>> + fprintf(out_fp, "Driver Trace Entries:\n");
>> + print_trace(hdr);
>> + fprintf(out_fp, "\n");
>> + break;
>> + case ID_DRV_TYPE:
>> + fprintf(out_fp, "Driver Type:\n");
>> + memcpy(&i, dump_map+hdr->offset, sizeof(i));
>> + i = TARGET_TO_HOST32(i, hdr->eye_catcher);
> Skip the memcpy and convert directly from dump_map+hdr->offset.
>
>> + fprintf(out_fp, "\t CCIN: 0x%04X\n", i);
>> + memcpy(&i, dump_map+hdr->offset+sizeof(i), sizeof(i));
>> + i = TARGET_TO_HOST32(i, hdr->eye_catcher);
> Likewise.
>
>> + fprintf(out_fp,"\t Adapter firmware Version: "
>> + " %02X:%02X:%02X:%02X\n\n",
>> + i >> 24, /* major release */
>> + (i & 0x00FF0000) >> 16, /* card type */
>> + (i & 0x0000FF00) >> 8, /* minor release 0 */
>> + i & 0x000000FF); /* minor release 1 */
>> + break;
>> + case ID_DRV_CTRL_BLK:
>> + case ID_DRV_PEND_OPS:
>> + /* not implemented yet */
>> + fprintf(stderr,
>> + "Warning: found IPR Control Block or IPR Pending Ops!\n"
>> + "These are not implemented yet. Skipping...\n");
>> + break;
>> + default:
>> + fprintf(stderr,
>> + "Warning: did not recognize data identification "
>> + "(id 0x%08X). Skipping...\n\n", hdr->id);
>> + }
>> +}
>> +
>> +/**
>> + * fsize -
>> + * @filename: ipr dump file name
>> + *
>> + * Returns:
>> + * ipr dump file size
>> + **/
>> +off_t fsize(const char *filename)
>> +{
>> + struct stat st;
>> +
>> + if (stat(filename, &st) == 0)
>> + return st.st_size;
>> +
>> + fprintf(stderr, "Cannot determine size of %s: %s\n",
>> + filename, strerror(errno));
>> +
>> + return -1;
>> +}
>> +
>> +/**
>> + * print_usage -
>> + *
>> + * Returns:
>> + * nothing
>> + **/
>> +static void print_usage()
>> +{
>> + fprintf(stdout,
>> + "Usage: iprdumpfmt [options] <dump_file> <report_file>\n"
>> + " Options:\n"
>> + "\t-h --help\tPrint this message\n"
>> + "\t-v --verbose\tPrint verbose trace data\n");
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + u32 i, offset;
>> + int in_fd;
>> + struct ipr_dump_header hdr;
>> + struct ipr_dump_entry_header entry_hdr;
>> +
>> + while (1) {
>> + static struct option long_options[] = {
>> + {"verbose", no_argument, &verbose_trace, 1},
>> + {"brief", no_argument, &verbose_trace, 0},
>> + {"help", no_argument, 0, 'h'},
>> + {0, 0, 0, 0}
>> + };
> Make this struct global. static keyword has a different meaning when
> used inside a function and there is no point in using it here.
>
>> + dump_map = mmap(NULL, fsize(argv[optind]), PROT_READ, MAP_PRIVATE,
>> in_fd, 0);
>> +
>> + read_dump_header(&hdr);
>> + print_dump_header(&hdr);
>> + fprintf(out_fp, "\n");
>> +
>> + if (dump_map == MAP_FAILED) {
>> + fprintf(stderr, "Could not map dump file to memory: %s\n",
>> + strerror(errno));
>> + return errno;
>> + }
> In read_dump_header you use dump_map, but you only check if the mmap
> failed afterwards. This will get you a segfault inside read_dump_header
> if the mapping fails. Do the verification right after the mmap, before
> doing anything else.
>
>> +
>> + offset = hdr.first_entry_offset;
>> + for (i = 0; i < hdr.num_entries; ++i) {
>> + offset = read_entry_header(&entry_hdr, offset);
>> + dump_entry_data(&entry_hdr);
>> + }
>> +
>> + return 0;
>> +}
Hi Gabriel,
Thanks again for the review! I'll fix the issues you pointed out and
send a proper patch afterwards.
--
Heitor Ricardo Alves de Siqueira
IBM Linux Technology Center
------------------------------------------------------------------------------
_______________________________________________
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel
