innconval and ORGANIZATION

[Julien ÉLIE]

Hi Kamil,

> %/usr/lib/news/bin/innconfval -s|grep ORGANIZA
> ORGANIZATION=''; export ORGANIZATION;
>
> And I think, that this is the reason that my innd inserts empty
> Organization: header to posts.
> I think that ORGANIZATION should be unset in this case.

That's interesting.  Thanks for having reported the issue.

What is happening is that we auto-override our setting.
According to the man page of inn.conf:

   organization
       What to put in the Organization: header if it is left blank
       by the poster.  The value of the ORGANIZATION environment
       variable, if set, overrides this setting.  The default is
       unset, which tells INN not to insert an Organization: header.

According to the innconfval documentation:

   String parameters that are set to NULL will be mapped to empty
   strings.  List values will be mapped to an array of strings.


I do not know what is the best thing to do.
I would like to change how innconfval behaves so that it does not
set NULL strings/lists to empty strings/lists.

The patch is straight-forward:

--- lib/innconf.c       (révision 8877)
+++ lib/innconf.c       (copie de travail)
@@ -750,6 +750,10 @@
     const char *letter;
     static const char tcl_unsafe[] = "$[]{}\"\\";

+    /* Do not output NULL values.  They are not empty strings. */
+    if (value == NULL)
+        return;
+
     switch (quoting) {
     case INNCONF_QUOTE_NONE:
         fprintf(file, "%s\n", value != NULL ? value : "");
@@ -804,6 +808,10 @@
     unsigned int i;
     static const char tcl_unsafe[] = "$[]{}\"\\";

+    /* Do not output NULL values.  They are not empty lists. */
+    if (value == NULL || value->strings == NULL)
+        return;
+
     switch (quoting) {
     case INNCONF_QUOTE_NONE:
         fprintf(file, "[ ");


With that patch, I confirm that the Organization: header field is
now not set by nnrpd (whereas it previously was, though undefined).


The only question is:  do people actually use the empty strings/lists
returned by innconfval?  I do not think they are really interesting...

Having grep'ed the whole source code of INN for innconfval, I believe
the change to innconfval has no impact on our scripts.
We call "innconfval pathhost" (in sendinpaths and send-ihave) which
is always defined.  inncheck calls "innconfval -C" which is fine.
And innshellvars + INN::Config only use paths, always defined.

Do you think it will hurt for shell, TCL and Perl scripts if they
use $variable which would be non-existing, instead of being set
to an empty string/list?

Such scripts should check if the variable is defined before using
it...  Especially when they can change of name.
I see that in our scripts, it is almost always to obtain a path
or the name of a program (pgp, uux, gzip, etc.).

As for shells, it might depend on the shell used...  No problem with
bash or zsh, at least.

With tclsh:
% echo $variable
can't read "variable": no such variable

Perl will send a warning if "use warnings" is on.  It does not stop
a script.  Anyway, it is bad coding to use a variable without checking
it.

I think that's OK to proceed with the change.
Any complaints about it?

-- 
Julien ÉLIE

« -- Essayons d'interroger ce garde habilement sans éveiller ses soupçons...
  -- Hep ! Où est enfermé Assurancetourix ? » (Astérix)