lprng-devel Mailing List for LPRng

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.

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

On Thu, Mar 19, 2009 at 06:12:32PM +0100, Bernhard R. Link wrote:
> Craig, could you submit the new pot (and new de.po, I needed something
> where I could test the plurals) to the translation project?
> Perhaps we get some more translations when people do not give when they
> reach those strings.
They're generally done at close to release time otherwise there you can
get translator exhaustion!

> (On the other hand, at least the current German translation is quite
> weak, and I guess the only thing to really help against thus would be
> many /* TRANSLATORS: ... */ descriptions of what message means what,
> currently there are just too many small snippets put together that
> it is hard to translate anything sensible without reading the code in
> my eyes).
I've never liked the way strings are made in lprng and yes I can see how
they can be nasty for a translator.  I prefer lots of complete sentences
that may even have part-duplication of other strings, but at least the
translator has some hope of translating it.

 - Craig

-- 
Craig Small      GnuPG:1C1B D893 1418 2AF4 45EE  95CB C76C E5AC 12CA DFA5
http://www.enc.com.au/                             csmall at : enc.com.au
http://www.debian.org/          Debian GNU/Linux, software should be Free 

Read_pid is never called unless with 3rd and 4th argument 0. Also most
users have the exactly same call pattern, so moving that into a new
function make the other functions a bit easier to read. (Especially as I
am trying to change some connection handling, where using the same fd
variable for different things in the same function tends to be a bit
confusing.)

Only non-triviality is that some code had pid = -1 per default, other =
0. This patch should contain everything to cope with that.

 common/getqueue.c     |   35 ++++++++++++++++++++++-------------
 common/lpd.c          |    8 +-------
 common/lpd_control.c  |   15 +++++----------
 common/lpd_dispatch.c |    8 ++------
 common/lpd_jobs.c     |    2 +-
 common/lpd_remove.c   |    9 ++-------
 common/lpd_status.c   |   18 +++++-------------
 include/getqueue.h    |    3 ++-
 8 files changed, 40 insertions(+), 58 deletions(-)

Hochachtungsvoll,
	Bernhard R. Link

Attached patch removes two compiler warnings from lpr.c
by removing the unused argument of Check_str_dup and the
trivial function Check_dup.

Anyone any objections?

common/lpr.c  |   39 ++++++++++++++-------------------------
include/lpr.h |    3 +--
2 files changed, 15 insertions(+), 27 deletions(-)

Hochachtungsvoll,
	Bernhard R. Link

* walter harms <wh...@bf...> [090322 15:05]:
> NTL i would prefer that you #ifdef the stuff. the reason is the popularity
> of the IPP. In future we may have someone that is willing to create the
> Service_ipp( talk, from_addr ); that is missing and we should help him so far
> we can.

Attached is a new version. #ifdef needs also a change to make_lpd_conf,
so that it ignores the resulting empty lines and not print "YYYY" alot.

Anyone any objections? Otherwise I'll commit.

Hochachtungsvoll,
	Bernhard R. Link

HI brl,

ich was thinking now for 2 days about your patch.
1. it is reasonable
 there is currently no IPP support an none of us has time to add it.

NTL i would prefer that you #ifdef the stuff. the reason is the popularity
of the IPP. In future we may have someone that is willing to create the
Service_ipp( talk, from_addr ); that is missing and we should help him so far
we can.


re,
 wh





Bernhard R. Link schrieb:
> I suggest to remove the IPP port support. I think there is almost no
> chance LPRng will ever get the code to support the protocol (and if
> it gets, that code is easily added back).
> 
> Currently this is just dead code and having the ports in the config
> index might misguide people to think there is support (where all LPRng
> currently supports is listening on the port but cannot answert IPP
> rquests but only LP requests which could never come there).
> 
>  ChangeLog                 |    1 +
>  src/common/lpd.c          |   20 --------------------
>  src/common/lpd_dispatch.c |   37 +------------------------------------
>  src/include/lp.h          |    2 --
>  src/vars.c                |    4 ----
>  5 files changed, 2 insertions(+), 62 deletions(-)
> 
> Hochachtungsvoll,
> 	Bernhard R. Link
> 
> 
> ------------------------------------------------------------------------
> 
> ------------------------------------------------------------------------------
> Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are
> powering Web 2.0 with engaging, cross-platform capabilities. Quickly and
> easily build your RIAs with Flex Builder, the Eclipse(TM)based development
> software that enables intelligent coding and step-through debugging.
> Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Lprng-devel mailing list
> Lpr...@li...
> https://lists.sourceforge.net/lists/listinfo/lprng-devel

I suggest to remove the IPP port support. I think there is almost no
chance LPRng will ever get the code to support the protocol (and if
it gets, that code is easily added back).

Currently this is just dead code and having the ports in the config
index might misguide people to think there is support (where all LPRng
currently supports is listening on the port but cannot answert IPP
rquests but only LP requests which could never come there).

 ChangeLog                 |    1 +
 src/common/lpd.c          |   20 --------------------
 src/common/lpd_dispatch.c |   37 +------------------------------------
 src/include/lp.h          |    2 --
 src/vars.c                |    4 ----
 5 files changed, 2 insertions(+), 62 deletions(-)

Hochachtungsvoll,
	Bernhard R. Link

I've applied the attach patch. I've looked at the translation,
and without plurals it was just not really nice.

Craig, could you submit the new pot (and new de.po, I needed something
where I could test the plurals) to the translation project?
Perhaps we get some more translations when people do not give when they
reach those strings.

Hochachtungsvoll,
	Bernhard R. Link

(On the other hand, at least the current German translation is quite
weak, and I guess the only thing to really help against thus would be
many /* TRANSLATORS: ... */ descriptions of what message means what,
currently there are just too many small snippets put together that
it is hard to translate anything sensible without reading the code in
my eyes).