Re: [Lprng-devel] [PATCH] simplify subserver forking code

hi Brl,
nice work !
i scaned the code (but did not apply the patch) and it looks nice.
i like the move from int pid -> pid_t pid
but i notice Service_worker() still has a int pid.


actually this code make me a bit nervous (fixed array size):



+pid_t Start_worker( const char *name, WorkerProc *proc, struct line_list *parms, int fd )
 {
+	struct line_list args;
+	int passfd[20];
+	pid_t pid;
+	int intern_fd = 0,
+	    intern_logger = -1,
+	    intern_status = -1,
+	    intern_mail = -1,
+	    intern_lpd = -1;
+	int passfd_count = 0;

+	passfd[passfd_count++] = 0;
+	passfd[passfd_count++] = 1;
+	passfd[passfd_count++] = 2;
+	if( Mail_fd > 0 ){
+		intern_mail = passfd_count;
+		passfd[passfd_count++] = Mail_fd;
+	}
+	if( Status_fd > 0 ){
+		intern_status = passfd_count;
+		passfd[passfd_count++] = Status_fd;
+	}
+	if( Logger_fd > 0 ){
+		intern_logger = passfd_count;
+		passfd[passfd_count++] = Logger_fd;
+	}
+	if( Lpd_request > 0 ){
+		intern_lpd = passfd_count;
+		passfd[passfd_count++] = Lpd_request;
+	}


then you call
+	pid = Make_lpd_call( name, proc, passfd_count, passfd, &args,
+			intern_logger, intern_status, intern_mail, intern_lpd,
+			intern_fd );


can you not simply set the unused values to -1 (or 0) ? and forget about intern_* ?
just providing  passfd_count, passfd ?
In the original code they were inside a struct { passfd_count, passfd } may you can revive the idea ?

Set unused fd to something useless has an other advantage, you can get rid of Lpd_request and friends
by replacing them with
#define Logger_fd  passfd[5]
(not checked, just an idea)

hope that helps my brain is toast today,
 wh





Bernhard R. Link schrieb:
> Attached patch simpliyfies the subserver forking code in lpd_worker.c
> (moved there from linelist.c). It moved the last users of
> Setup_lpd_call/Make_lpd_call to Start_worker, which was already used for
> most others. As those calls are thus intern to lpd_worker.c they are
> made static and simplified (not putting arrays of fds as pointers into
> string lists and making new fds explicit arguments, instead of
> converting them to strings and parsing them back again...
> 
>  ChangeLog                  |    1
>  src/common/lpd.c           |   88 ++++++++++-----------
>  src/common/lpd_dispatch.c  |   14 +--
>  src/common/lpd_jobs.c      |    8 -
>  src/common/lpd_logger.c    |   29 ++-----
>  src/common/lpd_worker.c    |  181 ++++++++++++++++++---------------------------
>  src/include/getqueue.h     |    4
>  src/include/linelist.h     |    2
>  src/include/lp.h           |    2
>  src/include/lpd.h          |    2
>  src/include/lpd_dispatch.h |    4
>  src/include/lpd_jobs.h     |    4
>  src/include/lpd_logger.h   |    3
>  src/include/lpd_worker.h   |    4
>  14 files changed, 145 insertions(+), 201 deletions(-)
> 
> Hochachtungsvoll,
> 	Bernhard R. Link
> 
> 
> ------------------------------------------------------------------------
> 
> This body part will be downloaded on demand.
> 
> 
> ------------------------------------------------------------------------
> 
> This body part will be downloaded on demand.