[PATCH] Utilities: clean up become_daemon

[Martin Schwenke]

Hi Andreas,

On Wed, 16 Aug 2017 16:54:10 +0200, Andreas Schneider <asn at samba.org>
wrote:

> On Wednesday, 16 August 2017 12:51:41 CEST Martin Schwenke via samba-technical 
> wrote:
> > We would like to use become_daemon in CTDB.  Here are some clean ups.
> > The only real functional change is the addition of some error checking
> > in become_daemon() itself, so that we notice fails fork() and setsid().  
> 
> Shouldn't
> 
> +
>  #include "lib/util/close_low_fd.h"
> +#include "lib/util/debug.h"
> +
> +#include "lib/util/samba_util.h"
> 
> be
> 
> #include "close_low_fd.h"
> #include "debug.h"
> #include "samba_util.h"
> 
> 
> They are all in the same directory as the C file itself ...

Sure.  Right now there are about 100 #includes in lib/util/ with
"lib/util/..." and about 150 local #includes without any directory.  I
guess it make sense to keep the local includes relative to the current
directory.

Updated patch attached...

> Also you remove _PUBLIC_ from several function which looks like they are not 
> exported as public functions anymore. So the symbols are not exported by the 
> library. Or was the _PUBLIC_ not defined anyway before?

I don't think _PUBLIC_ is needed anymore because any non-static
functions are automatically exported by libraries.  However, I'm
always happy to learn something if I'm wrong!  Only about 1/4 of files
in lib/util/ have _PUBLIC_ in them... and I have definitely removed it
in the past and nobody complained.  Less clutter!  ;-)

Compare these examples...

data_blob_clear_free() with _PUBLIC_:

$ git grep data_blob_clear_free lib/util/data_blob.*
lib/util/data_blob.c:_PUBLIC_ void data_blob_clear_free(DATA_BLOB *d)
lib/util/data_blob.h:_PUBLIC_ void data_blob_clear_free(DATA_BLOB *d);

become_daemon() without _PUBLIC_:

$ git grep 'void become_daemon' lib/util/become_daemon.*
lib/util/become_daemon.c:void become_daemon(bool do_fork, bool no_session, bool log_stdout)
lib/util/become_daemon.h:void become_daemon(bool do_fork, bool no_session, bool log_stdout);

They look to be the same sort of symbols:

$ objdump -t bin/default/lib/util/libsamba-util.so  | grep 'become_daemon$'
0000000000022d90 g     F .text	000000000000005b              become_daemon
$ objdump -t bin/default/lib/util/libsamba-util.so  | grep 'data_blob_clear_free$'
000000000001a780 g     F .text	0000000000000012              data_blob_clear_free

... and, more importantly, binaries link just fine against it.

peace & happiness,
martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: samba.patch
Type: text/x-patch
Size: 15702 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20170817/9bf40b7c/samba.bin>