lprng-devel Mailing List for LPRng

i think you missunderstood my intention.

Bernhard R. Link schrieb:
> * walter harms <wh...@bf...> [090330 19:08]:
>> i scaned the code (but did not apply the patch) and it looks nice.
>> [...]
>> actually this code make me a bit nervous [...]
> 
> What this code does is that it takes a list of fds that should be passed
> to the child process. This is done by dup()ing them and closing
> everything else, so the nth passed filedescriptor will have number n in
> the child (so they are all at the start with no holes in the
> allocation).
> 
>> can you not simply set the unused values to -1 (or 0) ? and forget about intern_* ?
> 
> This would mean mixing the code to handle the fd duping/closing with the
> meaning of those files. As the code is easy to get wrong (as the order
> of steps matters and sometimes it needs temporary copies to get
> everything right), this would make it much harder to get right and to
> read in my eyes.
> 

now we have the same info in two places one (local) array and global vars.
looking at the use of these globals you can see them used seldom.
it would requiere next to no changes since the code ist always checking
for *_fd >0
only the init and closing code have to set it to -1

>> (fixed array size)
> 
> Well, there are basicaly three options:
> 
> 1) Use a fixed array. As the number of items used is a static property
> of the code that is easy to see, there is none of any possible pitfalls
> of static arrays.
> 
> 2) Write your own implementation of a growing int list. As the numbers
> of elements is fixed, that is not only wasted code but also never
> tested...
> 
> 3) Misuse some other growing list implementation. (That was the old
> LPRng, everything was cast to pointers and back).
> 

> I'd rather have them as local as possible instead of putting them into
> some global array.

me too, but i would also like to reduce the number of variables putting them into a struct/array
is no way to reduce them but makes it more easy to pass them around and finding a way to remove
them alltogether without changing the code to much.



>> In the original code they were inside a struct { passfd_count, passfd } may you can revive the idea ?
> 
> what would either be 2) or just hiding stuff in the struct, so I see no
> advantage.
> 
	i my eyes the advantage is that they are together. the point of using a struct is
	that using  ->passfd  will automaticly requiere ->passfd_count.
	(note that this is a discussion point not a request)



>> 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)
> 

> 
> 
>> i like the move from int pid -> pid_t pid
>> but i notice Service_worker() still has a int pid.
> 
> Perhaps replacing them globally would be a nice task to tidy things up.
> Currently I mostly replace them wherever I change something (as some
> parts of the code already have pid_t and many not yet).
> 
> Hochachtungsvoll,
> 	Bernhard R. Link
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Lprng-devel mailing list
> Lpr...@li...
> https://lists.sourceforge.net/lists/listinfo/lprng-devel
> 
> 

* walter harms <wh...@bf...> [090330 19:08]:
> i scaned the code (but did not apply the patch) and it looks nice.
> [...]
> actually this code make me a bit nervous [...]

What this code does is that it takes a list of fds that should be passed
to the child process. This is done by dup()ing them and closing
everything else, so the nth passed filedescriptor will have number n in
the child (so they are all at the start with no holes in the
allocation).

> can you not simply set the unused values to -1 (or 0) ? and forget about intern_* ?

This would mean mixing the code to handle the fd duping/closing with the
meaning of those files. As the code is easy to get wrong (as the order
of steps matters and sometimes it needs temporary copies to get
everything right), this would make it much harder to get right and to
read in my eyes.

> (fixed array size)

Well, there are basicaly three options:

1) Use a fixed array. As the number of items used is a static property
of the code that is easy to see, there is none of any possible pitfalls
of static arrays.

2) Write your own implementation of a growing int list. As the numbers
of elements is fixed, that is not only wasted code but also never
tested...

3) Misuse some other growing list implementation. (That was the old
LPRng, everything was cast to pointers and back).

> In the original code they were inside a struct { passfd_count, passfd } may you can revive the idea ?

what would either be 2) or just hiding stuff in the struct, so I see no
advantage.

> 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)

I'd rather have them as local as possible instead of putting them into
some global array.


> i like the move from int pid -> pid_t pid
> but i notice Service_worker() still has a int pid.

Perhaps replacing them globally would be a nice task to tidy things up.
Currently I mostly replace them wherever I change something (as some
parts of the code already have pid_t and many not yet).

Hochachtungsvoll,
	Bernhard R. Link