Bug 6655 - [PATCH] smbcontrol smbd ping does not work
[PATCH] smbcontrol smbd ping does not work
Product: Samba 3.4
Classification: Unclassified
Component: Client Tools
All All
: P3 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2009-08-21 05:49 UTC by Olaf Flebbe
Modified: 2009-08-31 02:47 UTC (History)
0 users

See Also:

proposed patch (2.37 KB, patch)
2009-08-21 05:51 UTC, Olaf Flebbe
vl: review+
metze: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Olaf Flebbe 2009-08-21 05:49:29 UTC
1) Ping messages to smbd launched by

smbcontrol smbd ping

are answered by all daemons.

Essentialy it is the same as
smbcontrol all ping.


2) smbcontrol <largeinteger> ping

triggers weird behaviour. try 2147483649 for instance.

====== Analysis

1) is introduced as a side effect by change 64450cc1e441355aa8925b7183e90872eeab20b1
where all samba daemons annouce itself to the connections.tdb
This seems o.k. to me.

IMHO smbcontrol smbd ping should send a MSG_PING to the smbd alone, rather to the virtual process id meaning "broadcast".

Distinguishing between smbd/winbind(d)/nmbd and numerical pid was a little bit crude. So I left the work distinguishing between strings and arguments to interpret_pid().

smbd or nmbd seems to have different names in its pid file name, depending on its argument, so it is neccessary to change it here, too.

Looking at the code of interpret_pid() showed (2).


The pid member of server_id has the type pid_t. pid_t  is a signed quantity, so I changed the scanf to %d rather %u. Furthermore we should test for an integer overflow.
Comment 1 Olaf Flebbe 2009-08-21 05:49:48 UTC
Patch reworked after VL comment
Comment 2 Olaf Flebbe 2009-08-21 05:51:08 UTC
Created attachment 4581 [details]
proposed patch

reworked: pid_t may be a short
Comment 3 Olaf Flebbe 2009-08-21 06:13:57 UTC
IMHO Bug #4700 is fixed with this, too
Comment 4 Volker Lendecke 2009-08-24 09:19:13 UTC
Comment on attachment 4581 [details]
proposed patch

Pushed with some minor cosmetic changes: We use { } even for single-line if statements.

Comment 5 Volker Lendecke 2009-08-24 09:20:00 UTC
Karolin, I think we need a second review to get it into 3.4.1, right?

Comment 6 Stefan Metzmacher 2009-08-25 01:41:50 UTC
Comment on attachment 4581 [details]
proposed patch

Please cherry-pick
instead of using the attachment
Comment 7 Karolin Seeger 2009-08-31 02:47:11 UTC
Pushed, will be included in 3.4.1.
Closing out bug report.