Bug 6655 - [PATCH] smbcontrol smbd ping does not work
Summary: [PATCH] smbcontrol smbd ping does not work
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.4
Classification: Unclassified
Component: Client Tools (show other bugs)
Version: 3.4.0
Hardware: All All
: P3 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-08-21 05:49 UTC by Olaf Flebbe
Modified: 2009-08-31 02:47 UTC (History)
0 users

See Also:


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

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


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.

Volker
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?

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

Please cherry-pick
5359e397ff190c35414f6961be61a5110e237dd5
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.

Thanks!