[PATCH 1/4] last: fix memory leak

[PATCH 1/4] last: fix memory leak

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 login-utils/last.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/login-utils/last.c b/login-utils/last.c
index eb64bde..bc1b4ce 100644
--- a/login-utils/last.c
+++ b/login-utils/last.c
@@ -819,6 +819,10 @@ static void process_wtmp_file(const struct last_control *ctl)
 
 	printf(_("\n%s begins %s"), basename(ctl->altv[ctl->alti]), ctime(&begintime));
 	fclose(fp);
+	for (p = utmplist; p; p = next) {
+		next = p->next;
+		free(p);
+	}
 }
 
 int main(int argc, char **argv)
-- 
1.8.4

[PATCH 2/4] tools: generate autotools files if missing

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tools/config-gen | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/config-gen b/tools/config-gen
index 72cefc9..b7ccadd 100755
--- a/tools/config-gen
+++ b/tools/config-gen
@@ -28,5 +28,9 @@ if [ -n "$CFLAGS" ]; then
 	export CFLAGS
 fi
 
+if [ ! -f ./configure ]; then
+	./autogen.sh
+fi
+
 echo
 ./configure $opts
-- 
1.8.4

Re: [PATCH 2/4] tools: generate autotools files if missing

[Karel Zak]

On Sun, Sep 08, 2013 at 05:09:07PM +0100, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  tools/config-gen | 4 ++++
>  1 file changed, 4 insertions(+)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH 3/4] lib/fileutils: advice there is no point caching temporary files

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 lib/fileutils.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/fileutils.c b/lib/fileutils.c
index 92b474c..23cf80c 100644
--- a/lib/fileutils.c
+++ b/lib/fileutils.c
@@ -8,6 +8,7 @@
 #include <unistd.h>
 #include <sys/time.h>
 #include <sys/resource.h>
+#include <fcntl.h>
 
 #include "c.h"
 #include "fileutils.h"
@@ -43,6 +44,9 @@ int xmkstemp(char **tmpname, char *dir)
 		free(localtmp);
 		localtmp = NULL;
 	}
+#ifdef POSIX_FADV_NOREUSE
+	posix_fadvise(fd, 0, lseek(fd, 0, 0), POSIX_FADV_NOREUSE);
+#endif
 	*tmpname = localtmp;
 	return fd;
 }
-- 
1.8.4

Re: [PATCH 3/4] lib/fileutils: advice there is no point caching temporary files

[Pádraig Brady]

On 09/08/2013 05:09 PM, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  lib/fileutils.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/fileutils.c b/lib/fileutils.c
> index 92b474c..23cf80c 100644
> --- a/lib/fileutils.c
> +++ b/lib/fileutils.c
> @@ -8,6 +8,7 @@
>  #include <unistd.h>
>  #include <sys/time.h>
>  #include <sys/resource.h>
> +#include <fcntl.h>
>  
>  #include "c.h"
>  #include "fileutils.h"
> @@ -43,6 +44,9 @@ int xmkstemp(char **tmpname, char *dir)
>  		free(localtmp);
>  		localtmp = NULL;
>  	}
> +#ifdef POSIX_FADV_NOREUSE
> +	posix_fadvise(fd, 0, lseek(fd, 0, 0), POSIX_FADV_NOREUSE);
> +#endif
>  	*tmpname = localtmp;
>  	return fd;
>  }
> 

I don't follow. The above is saying to NOREUSE the data
from the start to the current position?
But won't the file be empty and the current position == 0 here?
Also POSIX_FADV_NOREUSE currently does nothing in the kernel.
There is POSIX_FADV_DONTNEED to drop the file (range) from cache.
But I'm not sure that's appropriate or needed for all temp files?
Note also DONTNEED is auto done when a file is unlinked.

cheers,
Pádraig.

Re: [PATCH 3/4] lib/fileutils: advice there is no point caching temporary files

[Sami Kerola]

On 9 September 2013 04:23, Pádraig Brady <P@draigbrady.com> wrote:
> On 09/08/2013 05:09 PM, Sami Kerola wrote:
>> +#ifdef POSIX_FADV_NOREUSE
>> +     posix_fadvise(fd, 0, lseek(fd, 0, 0), POSIX_FADV_NOREUSE);
>> +#endif
>
> I don't follow. The above is saying to NOREUSE the data
> from the start to the current position?
> But won't the file be empty and the current position == 0 here?
> Also POSIX_FADV_NOREUSE currently does nothing in the kernel.
> There is POSIX_FADV_DONTNEED to drop the file (range) from cache.
> But I'm not sure that's appropriate or needed for all temp files?
> Note also DONTNEED is auto done when a file is unlinked.

Hi Pádraig,

I misunderstood posix_fadvice() manual page. Thank you for explaining
how caching instructions work. Perhaps adding a note to
posix_fadvise(3P) about unlinking and DONOTNEED would be appropriate.
That, after all, makes manual fiddling with this function pointless in
cases like I proposed.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

[PATCH 4/4] hexdump: revert global exitval variable change

[Sami Kerola]

The change f2a037fb7b153954d5d34cca48182b6d8832fcfa had unfavorable
effect of making hexdump to return non-zero exit value always.

This happen because oversight when 'exitval' gets to be set.  By clance,
one might expect main() to call next() which will return value for
'exitval'.  That assessment misses later call chain main() -> display()
-> get() -> next(), which in reverse should return correct value for
'exitval'.

It was mentioned in util-linux maillist that Ondrej Oprala is working on
major renewal of the hexdump .  That in mind it seems best to simply to
revert the global 'exitval' and avoid conflict with Ondrej's work.

Reference: http://markmail.org/message/sbnvuhkboreujj5p
Reported-by: Dave Reisner <d@falconindy.com>
CC: Ondrej Oprala <ooprala@redhat.com>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 text-utils/display.c | 6 +++---
 text-utils/hexdump.c | 4 ++--
 text-utils/hexdump.h | 1 +
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/text-utils/display.c b/text-utils/display.c
index 41ddd8d..1f9a11b 100644
--- a/text-utils/display.c
+++ b/text-utils/display.c
@@ -295,7 +295,7 @@ get(void)
 int next(char **argv)
 {
 	static int done;
-	int statok, exitval = 0;
+	int statok;
 
 	if (argv) {
 		_argv = argv;
@@ -305,14 +305,14 @@ int next(char **argv)
 		if (*_argv) {
 			if (!(freopen(*_argv, "r", stdin))) {
 				warn("%s", *_argv);
-				exitval = 1;
+				exitval = EXIT_FAILURE;
 				++_argv;
 				continue;
 			}
 			statok = done = 1;
 		} else {
 			if (done++)
-				return(exitval);
+				return(0);
 			statok = 0;
 		}
 		if (skip)
diff --git a/text-utils/hexdump.c b/text-utils/hexdump.c
index 411d809..e966cc3 100644
--- a/text-utils/hexdump.c
+++ b/text-utils/hexdump.c
@@ -47,11 +47,11 @@
 
 FS *fshead;				/* head of format strings */
 ssize_t blocksize;			/* data block size */
+int exitval;				/* final exit value */
 ssize_t length = -1;			/* max bytes to read */
 
 int main(int argc, char **argv)
 {
-	int exitval;			/* final exit value */
 	FS *tfs;
 	char *p;
 
@@ -76,7 +76,7 @@ int main(int argc, char **argv)
 	for (tfs = fshead; tfs; tfs = tfs->nextfs)
 		rewrite(tfs);
 
-	exitval = next(argv);
+	(void)next(argv);
 	display();
 	return exitval;
 }
diff --git a/text-utils/hexdump.h b/text-utils/hexdump.h
index b2ea1f1..fa8f632 100644
--- a/text-utils/hexdump.h
+++ b/text-utils/hexdump.h
@@ -73,6 +73,7 @@ typedef struct _fs {			/* format strings */
 extern FU *endfu;
 extern FS *fshead;			/* head of format strings list */
 extern ssize_t blocksize;		/* data block size */
+extern int exitval;			/* final exit value */
 extern ssize_t length;			/* max bytes to read */
 extern off_t skip;                      /* bytes to skip */
 
-- 
1.8.4

Re: [PATCH 4/4] hexdump: revert global exitval variable change

[Karel Zak]

On Sun, Sep 08, 2013 at 05:09:09PM +0100, Sami Kerola wrote:
>  text-utils/display.c | 6 +++---
>  text-utils/hexdump.c | 4 ++--
>  text-utils/hexdump.h | 1 +
>  3 files changed, 6 insertions(+), 5 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 1/4] last: fix memory leak

[Karel Zak]

On Sun, Sep 08, 2013 at 05:09:06PM +0100, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  login-utils/last.c | 4 ++++
>  1 file changed, 4 insertions(+)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com