<shadow.h> function: fgetspent_r

Submitted by Markus Wichmann on Jan. 20, 2019, 3:41 p.m.

Details

Message ID 20190120154154.GA23924@voyager
State New
Series "<shadow.h> function: fgetspent_r"
Headers show

Commit Message

Markus Wichmann Jan. 20, 2019, 3:41 p.m.
Hi all,

so, I wrote a version of fgetspent_r() now. I based it on fgetspent().
For style, I adopted the Linux style -- might need to refactor that.
Returning EILSEQ on format error is a hack, but I found no better code.
As I said, glibc loops on error, but we do that for no other src/passwd
function, so we should either not start now or add that feature to every
other function.

One thing I noticed: If AccountService requires this interfaces, is it
possible that it doesn't support TCB shadow files?

Ciao,
Markus

Patch hide | download patch | download mbox

From 3489f9e5ef055c80464252fe640fead8aeb1068e Mon Sep 17 00:00:00 2001
From: Markus Wichmann <nullplan@gmx.net>
Date: Sun, 20 Jan 2019 16:31:34 +0100
Subject: [PATCH 5/5] Add fgetspent_r().

Interface was defined by glibc, and seems to have been adopted by
Solaris. Some freedesktop software appears to require it, and it adds
little bloat.

Added without feature test macros, since no other interface in shadow.h
requires it, even the ones documented to have required it in the past.
---
 include/shadow.h         |  1 +
 src/passwd/fgetspent_r.c | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)
 create mode 100644 src/passwd/fgetspent_r.c

diff --git a/include/shadow.h b/include/shadow.h
index 2b1be413..4edc90db 100644
--- a/include/shadow.h
+++ b/include/shadow.h
@@ -33,6 +33,7 @@  int putspent(const struct spwd *, FILE *);
 
 struct spwd *getspnam(const char *);
 int getspnam_r(const char *, struct spwd *, char *, size_t, struct spwd **);
+int fgetspent_r(FILE *f, struct spwd* sp, char *line, size_t size, struct spwd **spret);
 
 int lckpwdf(void);
 int ulckpwdf(void);
diff --git a/src/passwd/fgetspent_r.c b/src/passwd/fgetspent_r.c
new file mode 100644
index 00000000..643637de
--- /dev/null
+++ b/src/passwd/fgetspent_r.c
@@ -0,0 +1,27 @@ 
+#include "pwf.h"
+#include <pthread.h>
+#include <limits.h>
+#include <stdio.h>
+
+int fgetspent_r(FILE *f, struct spwd* sp, char *line, size_t size, struct spwd **spret)
+{
+	int res = 0;
+	int cs;
+	*spret = 0;
+	if (size > INT_MAX)
+		size = INT_MAX; //2GB ought to be enough for anyone
+	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
+	if (!fgets(line, size, f))
+		goto out;
+	res = ERANGE;
+	if (line[strlen(line) - 1] != '\n')
+		goto out;
+	res = EILSEQ;
+	if ( __parsespent(line, sp) < 0)
+		goto out;
+	*spret = sp;
+	res = 0;
+out:
+	pthread_setcancelstate(cs, 0);
+	return res;
+}
-- 
2.19.1


Comments

A. Wilcox Jan. 20, 2019, 9:12 p.m.
On 01/20/19 09:41, Markus Wichmann wrote:
> Hi all,
> 
> so, I wrote a version of fgetspent_r() now. I based it on fgetspent().
> For style, I adopted the Linux style -- might need to refactor that.
> Returning EILSEQ on format error is a hack, but I found no better code.
> As I said, glibc loops on error, but we do that for no other src/passwd
> function, so we should either not start now or add that feature to every
> other function.
> 
> One thing I noticed: If AccountService requires this interfaces, is it
> possible that it doesn't support TCB shadow files?
> 
> Ciao,
> Markus


It doesn't support TCB shadow files.


> +int fgetspent_r(FILE *f, struct spwd* sp, char *line, size_t size,
struct spwd **spret)

Tiny style nit: I think the * is meant to be kept with 'sp', not 'spwd',
in the second argument.


Thank you so much for this!  I will test this patch out ASAP and report
how well it works.

Best,
--arw
A. Wilcox Jan. 20, 2019, 10:02 p.m.
On 01/20/19 09:41, Markus Wichmann wrote:
> +	int res = 0;

This needs to be EIO; 0 means success, and causes AccountsService to
dereference the NULL that it receives when the file is exhausted (no
more lines).

Other than this, basic functionality seems to work.  I need to port
KDE's User Manager from logind to ConsoleKit2 before I can test it
fully, but listing (the part that uses fgetspent_r) gives the correct list.

Thank you again!

Best to you and yours,
--arw
Rich Felker Jan. 21, 2019, 12:50 a.m.
On Sun, Jan 20, 2019 at 03:12:59PM -0600, A. Wilcox wrote:
> On 01/20/19 09:41, Markus Wichmann wrote:
> > Hi all,
> > 
> > so, I wrote a version of fgetspent_r() now. I based it on fgetspent().
> > For style, I adopted the Linux style -- might need to refactor that.
> > Returning EILSEQ on format error is a hack, but I found no better code.
> > As I said, glibc loops on error, but we do that for no other src/passwd
> > function, so we should either not start now or add that feature to every
> > other function.
> > 
> > One thing I noticed: If AccountService requires this interfaces, is it
> > possible that it doesn't support TCB shadow files?
> > 
> > Ciao,
> > Markus
> 
> 
> It doesn't support TCB shadow files.

How so? The whole point of this interface is that it reads from a
FILE* provided by the caller, not the system shadow store. The usage
case is writing utilities that access and modify the underlying files
(or possibly tempfile copies thereof).

> > +int fgetspent_r(FILE *f, struct spwd* sp, char *line, size_t size,
> struct spwd **spret)
> 
> Tiny style nit: I think the * is meant to be kept with 'sp', not 'spwd',
> in the second argument.

Yes.

Rich