[Chilli] [PATCH] Binconfigfile not moved after chilliredir fork - Rev. 257

[Alberto Bellettato]

I dug into the code, not easy to understand the flow of the code with the forks and the binconfig passing form one to another, personally I would 
not have choosen a so articulated path...

I figured out that the main chilli forks a process chilli_opt to create the binconfig file, then it changes the directory of the binconfig to match
its own parent PID, it makes all kind of stuff, including the forking of the chilli_redir and others sub-programs like that. Is that correct?
However I do not understand how the chilli_opt PID can be less than the main chilli one... usually a child forked process has a greater number than
the parent...

Anyhow I succeded in having the binconfig file in the right /tmp/chilli-PID directory and the child process [chilli_redir] using the same file
with the "-b".

Here it is an example:
# ps|grep chilli
5800 root      5304 S    /usr/sbin/chilli 
5806 root      4236 S    [chilli_redir] -b /tmp/chilli-5800/config.bin 

# ls /tmp
chilli-5800

# ls /tmp/chilli-5800
config.bin

The patch follows.

HOWEVER 2 problems are still there:

1. Inside the config.bin there is the wrong binconfig file path (and I am not suprised about it because the file generated with the chilli_opt's PID
                                                                 has been moved by rename() to the new directory without recreating it )
Example (in the same context of the previous ones, showing only a part of the output):
# cat /tmp/chilli-5800/config.bin
....../tmp/chilli-5797/config.bin/var/run/chilli.pid192.168.182.0/24/255.255.255.0.......

I suppose the PID 5797 is the chilli_opt's one, that created the binfile just before the chilli server started and moved the file to the new dir.

Does this impact on some functionality?? I.E. is the binconfig path read from the file itself for some weird reason?
I saw that in the previous revisions it was not written to the file.

2. The command line parameter -b (or --bin) does not work. For example specifying --bin=/tmp/test.bin has no effect.
   Looking at the code I was supposing that after chilli_opt calls chilli_binconfig() passing to it pid 0, it should have got the _option.binconfig 
   value (bc var), that was passed through configuration or command line.
   However debugging the code I discovered that _option.binconfig is always set to /tmp/chilli-PID_OF_CHILLI_OPT/config.bin. How is it possible??
   I can not find the point where that happens.

3. Sometimes the /tmp/chilli-xxxx dir is not deleted (usually it does)


Index: src/chilli.c
===================================================================
--- src/chilli.c	(revisione 259)
+++ src/chilli.c	(copia locale)
@@ -152,11 +152,13 @@
 
 int chilli_binconfig(char *file, size_t flen, pid_t pid) {
   char * bc = _options.binconfig;
-  if (bc) {
-    return snprintf(file, flen, "%s", bc);
-  } else {
-    if (pid == 0) pid = chilli_pid;
-    if (pid == 0) pid = getpid();
+  if (pid == 0) {
+    if (bc) {
+      return snprintf(file, flen, "%s", bc);
+    } else {
+      pid = chilli_pid;
+      if (pid == 0) pid = getpid();
+    }
   }
   return snprintf(file, flen, "/tmp/chilli-%d/config.bin", pid);
 }
@@ -4170,7 +4172,7 @@
 	log_err(errno, file2);
 
       chilli_binconfig(file, sizeof(file), cpid);
-      snprintf(file2, sizeof(file2), "/tmp/chilli-%d/config.bin", getpid());
+      chilli_binconfig(file2, sizeof(file2), getpid());
 
       if (rename(file, file2)) log_err(errno, file);



Il giorno mar, 15/12/2009 alle 19.44 +0100, Alberto Bellettato ha
scritto:
> Sorry, too quick submitting the patch.. the code is NOT working fine
> yet.
> There is still a problem with [chill_redir] child staying linked to the
> old config.bin directory.
> 
> If I have got time tomorrow I will dig the code..
> 
> 
> 
> > In rev. 257 there is a problem with the binstatusfile, that issues a message in the log files "(Directory not empty) /tmp/chilli-xxxxx".
> > 
> > That is caused by the rename(file,file2) not working (not moving the bin file) because file = file2 = file with old PID.
> > Infact file2 is set by chilli_binconfig(), that used the "bc = _options.binconfig" value (set to the old PID).
> > 
> > I think the /tmp/chilli-xxxxx dir should be clean when chilli stop too, but it is not a priority since it uses only 16 bytes.
> > 
> > Here is the patch.
> > 
> > Index: src/chilli.c
> > ===================================================================
> > --- src/chilli.c        (revisione 257)
> > +++ src/chilli.c        (copia locale)
> > @@ -4170,9 +4170,9 @@
> >         log_err(errno, file2);
> > 
> >        chilli_binconfig(file, sizeof(file), cpid);
> > -      chilli_binconfig(file2, sizeof(file2), getpid());
> > +      snprintf(file2, sizeof(file2), "/tmp/chilli-%d/config.bin", getpid());
> > 
> > -      rename(file, file2);
> > +      if (rename(file, file2)) log_err(errno, file);
> > 
> >        umask(process_mask);
> > 
>