* [PATCH 0/4] Tools: hv @ 2012-09-05 17:01 K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 1/4] tools: hv: Fix file handle leak K. Y. Srinivasan 2012-09-05 19:11 ` [PATCH 0/4] Tools: hv Greg KH 0 siblings, 2 replies; 11+ messages in thread From: K. Y. Srinivasan @ 2012-09-05 17:01 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw Cc: K. Y. Srinivasan This patchset was authored by Ben Hutchings <ben@decadent.org.uk>. Ben asked me to submit this patchset. This patchset cleans up the KVP daemon code and fixes some bugs. K. Y. Srinivasan (4): tools: hv: Fix file handle leak tools: hv: Fix exit() error code tools: hv: Check for read/write errors tools: hv: Parse /etc/os-release tools/hv/hv_kvp_daemon.c | 95 +++++++++++++++++++++++++++++++++++++-------- 1 files changed, 78 insertions(+), 17 deletions(-) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] tools: hv: Fix file handle leak 2012-09-05 17:01 [PATCH 0/4] Tools: hv K. Y. Srinivasan @ 2012-09-05 17:02 ` K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 2/4] tools: hv: Fix exit() error code K. Y. Srinivasan ` (2 more replies) 2012-09-05 19:11 ` [PATCH 0/4] Tools: hv Greg KH 1 sibling, 3 replies; 11+ messages in thread From: K. Y. Srinivasan @ 2012-09-05 17:02 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw Cc: K. Y. Srinivasan, stable Match up each fopen() with an fclose(). Signed-off-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: stable@vger.kernel.org --- tools/hv/hv_kvp_daemon.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index c8e1013..4514fb4 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -160,7 +160,7 @@ static void kvp_update_file(int pool) sizeof(struct kvp_record), kvp_file_info[pool].num_records, filep); - fflush(filep); + fclose(filep); kvp_release_lock(pool); } @@ -207,6 +207,7 @@ static void kvp_update_mem_state(int pool) kvp_file_info[pool].records = record; kvp_file_info[pool].num_records = records_read; + fclose(filep); kvp_release_lock(pool); } static int kvp_file_init(void) -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] tools: hv: Fix exit() error code 2012-09-05 17:02 ` [PATCH 1/4] tools: hv: Fix file handle leak K. Y. Srinivasan @ 2012-09-05 17:02 ` K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 3/4] tools: hv: Check for read/write errors K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 4/4] tools: hv: Parse /etc/os-release K. Y. Srinivasan 2 siblings, 0 replies; 11+ messages in thread From: K. Y. Srinivasan @ 2012-09-05 17:02 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw Cc: K. Y. Srinivasan, stable Linux native exit codes are 8-bit unsigned values. exit(-1) results in an exit code of 255, which is usually reserved for shells reporting 'command not found'. Use the portable value EXIT_FAILURE. (Not that this matters much for a daemon.) Signed-off-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: stable@vger.kernel.org --- tools/hv/hv_kvp_daemon.c | 22 +++++++++++----------- 1 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 4514fb4..01b3ca5 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -122,7 +122,7 @@ static void kvp_acquire_lock(int pool) if (fcntl(kvp_file_info[pool].fd, F_SETLKW, &fl) == -1) { syslog(LOG_ERR, "Failed to acquire the lock pool: %d", pool); - exit(-1); + exit(EXIT_FAILURE); } } @@ -134,7 +134,7 @@ static void kvp_release_lock(int pool) if (fcntl(kvp_file_info[pool].fd, F_SETLK, &fl) == -1) { perror("fcntl"); syslog(LOG_ERR, "Failed to release the lock pool: %d", pool); - exit(-1); + exit(EXIT_FAILURE); } } @@ -153,7 +153,7 @@ static void kvp_update_file(int pool) if (!filep) { kvp_release_lock(pool); syslog(LOG_ERR, "Failed to open file, pool: %d", pool); - exit(-1); + exit(EXIT_FAILURE); } bytes_written = fwrite(kvp_file_info[pool].records, @@ -179,7 +179,7 @@ static void kvp_update_mem_state(int pool) if (!filep) { kvp_release_lock(pool); syslog(LOG_ERR, "Failed to open file, pool: %d", pool); - exit(-1); + exit(EXIT_FAILURE); } while (!feof(filep)) { readp = &record[records_read]; @@ -196,7 +196,7 @@ static void kvp_update_mem_state(int pool) if (record == NULL) { syslog(LOG_ERR, "malloc failed"); - exit(-1); + exit(EXIT_FAILURE); } continue; } @@ -225,7 +225,7 @@ static int kvp_file_init(void) if (access("/var/opt/hyperv", F_OK)) { if (mkdir("/var/opt/hyperv", S_IRUSR | S_IWUSR | S_IROTH)) { syslog(LOG_ERR, " Failed to create /var/opt/hyperv"); - exit(-1); + exit(EXIT_FAILURE); } } @@ -1358,13 +1358,13 @@ int main(void) if (kvp_file_init()) { syslog(LOG_ERR, "Failed to initialize the pools"); - exit(-1); + exit(EXIT_FAILURE); } fd = socket(AF_NETLINK, SOCK_DGRAM, NETLINK_CONNECTOR); if (fd < 0) { syslog(LOG_ERR, "netlink socket creation failed; error:%d", fd); - exit(-1); + exit(EXIT_FAILURE); } addr.nl_family = AF_NETLINK; addr.nl_pad = 0; @@ -1376,7 +1376,7 @@ int main(void) if (error < 0) { syslog(LOG_ERR, "bind failed; error:%d", error); close(fd); - exit(-1); + exit(EXIT_FAILURE); } sock_opt = addr.nl_groups; setsockopt(fd, 270, 1, &sock_opt, sizeof(sock_opt)); @@ -1396,7 +1396,7 @@ int main(void) if (len < 0) { syslog(LOG_ERR, "netlink_send failed; error:%d", len); close(fd); - exit(-1); + exit(EXIT_FAILURE); } pfd.fd = fd; @@ -1608,7 +1608,7 @@ kvp_done: len = netlink_send(fd, incoming_cn_msg); if (len < 0) { syslog(LOG_ERR, "net_link send failed; error:%d", len); - exit(-1); + exit(EXIT_FAILURE); } } -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] tools: hv: Check for read/write errors 2012-09-05 17:02 ` [PATCH 1/4] tools: hv: Fix file handle leak K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 2/4] tools: hv: Fix exit() error code K. Y. Srinivasan @ 2012-09-05 17:02 ` K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 4/4] tools: hv: Parse /etc/os-release K. Y. Srinivasan 2 siblings, 0 replies; 11+ messages in thread From: K. Y. Srinivasan @ 2012-09-05 17:02 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw Cc: K. Y. Srinivasan, stable hv_kvp_daemon currently does not check whether fread() or fwrite() succeed. Add the necessary checks. Also, remove the incorrect use of feof() before fread(). Signed-off-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: stable@vger.kernel.org --- tools/hv/hv_kvp_daemon.c | 22 +++++++++++++++++++--- 1 files changed, 19 insertions(+), 3 deletions(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 01b3ca5..3922abc 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -160,7 +160,12 @@ static void kvp_update_file(int pool) sizeof(struct kvp_record), kvp_file_info[pool].num_records, filep); - fclose(filep); + if (ferror(filep) || fclose(filep)) { + kvp_release_lock(pool); + syslog(LOG_ERR, "Failed to write file, pool: %d", pool); + exit(EXIT_FAILURE); + } + kvp_release_lock(pool); } @@ -181,12 +186,17 @@ static void kvp_update_mem_state(int pool) syslog(LOG_ERR, "Failed to open file, pool: %d", pool); exit(EXIT_FAILURE); } - while (!feof(filep)) { + for (;;) { readp = &record[records_read]; records_read += fread(readp, sizeof(struct kvp_record), ENTRIES_PER_BLOCK * num_blocks, filep); + if (ferror(filep)) { + syslog(LOG_ERR, "Failed to read file, pool: %d", pool); + exit(EXIT_FAILURE); + } + if (!feof(filep)) { /* * We have more data to read. @@ -249,12 +259,18 @@ static int kvp_file_init(void) fclose(filep); return 1; } - while (!feof(filep)) { + for (;;) { readp = &record[records_read]; records_read += fread(readp, sizeof(struct kvp_record), ENTRIES_PER_BLOCK, filep); + if (ferror(filep)) { + syslog(LOG_ERR, "Failed to read file, pool: %d", + i); + exit(EXIT_FAILURE); + } + if (!feof(filep)) { /* * We have more data to read. -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] tools: hv: Parse /etc/os-release 2012-09-05 17:02 ` [PATCH 1/4] tools: hv: Fix file handle leak K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 2/4] tools: hv: Fix exit() error code K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 3/4] tools: hv: Check for read/write errors K. Y. Srinivasan @ 2012-09-05 17:02 ` K. Y. Srinivasan 2012-09-05 16:58 ` Alan Cox 2 siblings, 1 reply; 11+ messages in thread From: K. Y. Srinivasan @ 2012-09-05 17:02 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw Cc: K. Y. Srinivasan, stable There is a new convention, used by systemd and supported by most distributions, to put basic OS release information in /etc/os-release. Signed-off-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: stable@vger.kernel.org --- tools/hv/hv_kvp_daemon.c | 50 +++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 47 insertions(+), 3 deletions(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index 3922abc..77a8413 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -465,15 +465,59 @@ void kvp_get_os_info(void) if (p) *p = '\0'; + /* + * Parse the /etc/os-release file if present: + * http://www.freedesktop.org/software/systemd/man/os-release.html + */ + file = fopen("/etc/os-release", "r"); + if (file != NULL) { + while (fgets(buf, sizeof(buf), file)) { + char *value, *q; + + /* Ignore comments */ + if (buf[0] == '#') + continue; + + /* Split into name=value */ + p = strchr(buf, '='); + if (!p) + continue; + *p++ = 0; + + /* Remove quotes and newline; un-escape */ + value = p; + q = p; + while (*p) { + if (*p == '\\') { + ++p; + if (!*p) + break; + *q++ = *p++; + } else if (*p == '\'' || *p == '"' || + *p == '\n') { + ++p; + } else { + *q++ = *p++; + } + } + *q = 0; + + if (!strcmp(buf, "NAME")) + os_name = strdup(value); + else if (!strcmp(buf, "VERSION_ID")) + os_major = strdup(value); + } + fclose(file); + return; + } + + /* Fallback for older RH/SUSE releases */ file = fopen("/etc/SuSE-release", "r"); if (file != NULL) goto kvp_osinfo_found; file = fopen("/etc/redhat-release", "r"); if (file != NULL) goto kvp_osinfo_found; - /* - * Add code for other supported platforms. - */ /* * We don't have information about the os. -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] tools: hv: Parse /etc/os-release 2012-09-05 17:02 ` [PATCH 4/4] tools: hv: Parse /etc/os-release K. Y. Srinivasan @ 2012-09-05 16:58 ` Alan Cox 2012-09-05 18:16 ` KY Srinivasan 0 siblings, 1 reply; 11+ messages in thread From: Alan Cox @ 2012-09-05 16:58 UTC (permalink / raw) To: K. Y. Srinivasan Cc: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw > + if (!strcmp(buf, "NAME")) > + os_name = strdup(value); > + else if (!strcmp(buf, "VERSION_ID")) > + os_major = strdup(value); strdup can fail. The case where it leaks because NAME= occurs twice is harmless enough but the strdup ought to be checked or add an x_strdup() helper. Alan ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 4/4] tools: hv: Parse /etc/os-release 2012-09-05 16:58 ` Alan Cox @ 2012-09-05 18:16 ` KY Srinivasan 0 siblings, 0 replies; 11+ messages in thread From: KY Srinivasan @ 2012-09-05 18:16 UTC (permalink / raw) To: Alan Cox; +Cc: gregkh, linux-kernel, devel, olaf, apw, ben, thozza, dcbw > -----Original Message----- > From: Alan Cox [mailto:alan@lxorguk.ukuu.org.uk] > Sent: Wednesday, September 05, 2012 12:58 PM > To: KY Srinivasan > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com; > ben@decadent.org.uk; thozza@redhat.com; dcbw@redhat.com > Subject: Re: [PATCH 4/4] tools: hv: Parse /etc/os-release > > > + if (!strcmp(buf, "NAME")) > > + os_name = strdup(value); > > + else if (!strcmp(buf, "VERSION_ID")) > > + os_major = strdup(value); > > strdup can fail. The case where it leaks because NAME= occurs twice is > harmless enough but the strdup ought to be checked or add an x_strdup() > helper. I will fix this and resend this patch. Thank you, K. Y ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Tools: hv 2012-09-05 17:01 [PATCH 0/4] Tools: hv K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 1/4] tools: hv: Fix file handle leak K. Y. Srinivasan @ 2012-09-05 19:11 ` Greg KH 2012-09-05 19:24 ` KY Srinivasan 1 sibling, 1 reply; 11+ messages in thread From: Greg KH @ 2012-09-05 19:11 UTC (permalink / raw) To: K. Y. Srinivasan; +Cc: linux-kernel, devel, olaf, apw, ben, thozza, dcbw On Wed, Sep 05, 2012 at 10:01:59AM -0700, K. Y. Srinivasan wrote: > This patchset was authored by Ben Hutchings <ben@decadent.org.uk>. Ben > asked me to submit this patchset. This patchset cleans up the KVP daemon > code and fixes some bugs. > > > K. Y. Srinivasan (4): > tools: hv: Fix file handle leak > tools: hv: Fix exit() error code > tools: hv: Check for read/write errors > tools: hv: Parse /etc/os-release If Ben wrote these, why are you showing that you wrote them? Please ALWAYS keep the proper authorship information on ALL patches. To not do so is rude and problematic on on so many different levels. Fix this up and resend please. greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 0/4] Tools: hv 2012-09-05 19:11 ` [PATCH 0/4] Tools: hv Greg KH @ 2012-09-05 19:24 ` KY Srinivasan 2012-09-05 19:38 ` Greg KH 0 siblings, 1 reply; 11+ messages in thread From: KY Srinivasan @ 2012-09-05 19:24 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, devel, olaf, apw, ben, thozza, dcbw > -----Original Message----- > From: Greg KH [mailto:gregkh@linuxfoundation.org] > Sent: Wednesday, September 05, 2012 3:11 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com; > dcbw@redhat.com > Subject: Re: [PATCH 0/4] Tools: hv > > On Wed, Sep 05, 2012 at 10:01:59AM -0700, K. Y. Srinivasan wrote: > > This patchset was authored by Ben Hutchings <ben@decadent.org.uk>. Ben > > asked me to submit this patchset. This patchset cleans up the KVP daemon > > code and fixes some bugs. > > > > > > K. Y. Srinivasan (4): > > tools: hv: Fix file handle leak > > tools: hv: Fix exit() error code > > tools: hv: Check for read/write errors > > tools: hv: Parse /etc/os-release > > If Ben wrote these, why are you showing that you wrote them? Ben asked me to apply these patches on the tip (of my tree) and send the patches out and that is what I did. I recreated these patches after applying to my tree and that is what I have sent out. How could I have made the authorship more clear (other than noting so in the comment which I have done). Regards, K. Y ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Tools: hv 2012-09-05 19:24 ` KY Srinivasan @ 2012-09-05 19:38 ` Greg KH 2012-09-05 19:49 ` KY Srinivasan 0 siblings, 1 reply; 11+ messages in thread From: Greg KH @ 2012-09-05 19:38 UTC (permalink / raw) To: KY Srinivasan; +Cc: linux-kernel, devel, olaf, apw, ben, thozza, dcbw On Wed, Sep 05, 2012 at 07:24:37PM +0000, KY Srinivasan wrote: > > On Wed, Sep 05, 2012 at 10:01:59AM -0700, K. Y. Srinivasan wrote: > > > This patchset was authored by Ben Hutchings <ben@decadent.org.uk>. Ben > > > asked me to submit this patchset. This patchset cleans up the KVP daemon > > > code and fixes some bugs. > > > > > > > > > K. Y. Srinivasan (4): > > > tools: hv: Fix file handle leak > > > tools: hv: Fix exit() error code > > > tools: hv: Check for read/write errors > > > tools: hv: Parse /etc/os-release > > > > If Ben wrote these, why are you showing that you wrote them? > > Ben asked me to apply these patches on the tip (of my tree) and send the > patches out and that is what I did. I recreated these patches after applying to > my tree and that is what I have sent out. How could I have made the authorship > more clear (other than noting so in the comment which I have done). You could have actually _kept_ the proper authorship information so that git shows who wrote the patch. As your shortlog shows above, it looks like you wrote the patches, not Ben, which is not acceptable. As to how to actually do that, well, I'll leave that as an exercise for the reader (hint, we document how to do this quite well...) greg k-h ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 0/4] Tools: hv 2012-09-05 19:38 ` Greg KH @ 2012-09-05 19:49 ` KY Srinivasan 0 siblings, 0 replies; 11+ messages in thread From: KY Srinivasan @ 2012-09-05 19:49 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, devel, olaf, apw, ben, thozza, dcbw > -----Original Message----- > From: Greg KH [mailto:gregkh@linuxfoundation.org] > Sent: Wednesday, September 05, 2012 3:39 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de; > apw@canonical.com; ben@decadent.org.uk; thozza@redhat.com; > dcbw@redhat.com > Subject: Re: [PATCH 0/4] Tools: hv > > On Wed, Sep 05, 2012 at 07:24:37PM +0000, KY Srinivasan wrote: > > > On Wed, Sep 05, 2012 at 10:01:59AM -0700, K. Y. Srinivasan wrote: > > > > This patchset was authored by Ben Hutchings <ben@decadent.org.uk>. Ben > > > > asked me to submit this patchset. This patchset cleans up the KVP daemon > > > > code and fixes some bugs. > > > > > > > > > > > > K. Y. Srinivasan (4): > > > > tools: hv: Fix file handle leak > > > > tools: hv: Fix exit() error code > > > > tools: hv: Check for read/write errors > > > > tools: hv: Parse /etc/os-release > > > > > > If Ben wrote these, why are you showing that you wrote them? > > > > Ben asked me to apply these patches on the tip (of my tree) and send the > > patches out and that is what I did. I recreated these patches after applying to > > my tree and that is what I have sent out. How could I have made the authorship > > more clear (other than noting so in the comment which I have done). > > You could have actually _kept_ the proper authorship information so that > git shows who wrote the patch. As your shortlog shows above, it looks > like you wrote the patches, not Ben, which is not acceptable. Obviously, I did not know how I could have done this. I will figure out the correct way of doing this and resubmit these patches. Thanks, K. Y ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-09-05 19:49 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-05 17:01 [PATCH 0/4] Tools: hv K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 1/4] tools: hv: Fix file handle leak K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 2/4] tools: hv: Fix exit() error code K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 3/4] tools: hv: Check for read/write errors K. Y. Srinivasan 2012-09-05 17:02 ` [PATCH 4/4] tools: hv: Parse /etc/os-release K. Y. Srinivasan 2012-09-05 16:58 ` Alan Cox 2012-09-05 18:16 ` KY Srinivasan 2012-09-05 19:11 ` [PATCH 0/4] Tools: hv Greg KH 2012-09-05 19:24 ` KY Srinivasan 2012-09-05 19:38 ` Greg KH 2012-09-05 19:49 ` KY Srinivasan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).