lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 4 May 2011 11:02:52 -0700 From: Anirban Chakraborty <anirban.chakraborty@...gic.com> To: Ben Hutchings <bhutchings@...arflare.com> CC: netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net> Subject: Re: [ethtool PATCH] FW dump support On May 4, 2011, at 10:40 AM, Ben Hutchings wrote: > On Mon, 2011-05-02 at 16:29 -0700, anirban.chakraborty@...gic.com wrote: >> From: Anirban Chakraborty <anirban.chakraborty@...gic.com> >> >> Added support to take FW dump via ethtool. > [...] >> --- a/ethtool.c >> +++ b/ethtool.c > [...] >> @@ -263,6 +270,12 @@ static struct option { >> "Get Rx ntuple filters and actions\n" }, >> { "-P", "--show-permaddr", MODE_PERMADDR, >> "Show permanent hardware address" }, >> + { "-W", "--get-dump", MODE_GET_DUMP, >> + "Get dump level\n" }, >> + { "-Wd", "--get-dump-data", MODE_GET_DUMP_DATA, >> + "Get dump data", "FILENAME " "Name of the dump file\n" }, > > The short options should only include one letter. Also the general > pattern is that 'get' options use lower-case letters and 'set' options > use upper-case letters. No, I'm not sure how best to handle a set of 3 > options. Maybe you can combine --get-dump and --get-dump-data, making > the filename optional? Thanks for the info, I was not aware of it. Will address it. > >> + { "-w", "--set-dump", MODE_SET_DUMP, >> + "Set dump level", "DUMPLEVEL " "Dump level for the device\n" }, > > The field this sets is described as 'flags' so does it consist of flags > or is it a level? The idea is to have an opaque flag for the driver that it uses to set the dump level (and hence the size) of it. I will use flag instead of level here so that there is no ambiguity. > > [...] >> @@ -3241,6 +3270,86 @@ static int do_grxntuple(int fd, struct ifreq *ifr) >> return 0; >> } >> >> +static void do_writedump(struct ethtool_dump *dump) >> +{ >> + FILE *f = fopen(dump_file, "wb+"); >> + size_t bytes; >> + >> + if (!f ) { >> + fprintf(stderr, "Can't open file %s: %s\n", >> + dump_file, strerror(errno)); >> + return; > > On error, we must exit with code 1. Will do. > >> + } >> + >> + bytes = fwrite(dump->data, 1, dump->len, f); >> + fclose(f); > [...] > > These functions can also fail and need to be checked. (Yes, fclose() > can fail, since it may have to flush buffered data.) I agree. Will fix it. Thanks for the feedback. -Anirban -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists