Update ipc.c

[LouisAtGH]

These changes repair the communication between pim and pimctl. ^pimctl-commands^ are working now. There a few command related issues left see issue log.

There are two issues left:

• Sometimes, relatively often, there arise is a space character before the entered command, with as consequence that the command is not recognized. Examle: Nov 3 18:45:47 pfSense pimd[48699]: Failed reading command from client: Bad message Nov 3 18:45:47 pfSense pimd[48699]: After 'for-statemend' => 'Bad cmd' ^ compat^

• one command is passing the ^if (!strncasecmp(cmd, c->cmd, len))^twice. First time with command "help" and second time with the real command. Never the less it works :) Example: Nov 3 18:40:52 pfSense pimd[48699]: Command Id: 1, len: 4, msg: ^help^ Nov 3 18:40:52 pfSense pimd[48699]: Command Id: 11, len: 17, msg: ^show pim neighbor^

[troglobit]

You're new at this, so here's a quick guide that applies to most projects, not just pimd. A pull request (or patch) to a project should:

1. be as small as possible
2. not change coding style in the change or surrounding code
3. not introduce whitespace changes in the same commit as a logical/functional change
4. not introduce unwarrented logging
5. not introduce commented-out code (i.e., dead code), e.g. temporary code used for debugging

I've spent a few minutes cleaning up your commit, removing cruft, random coding style changes, and what not. Reducing your code down to what under normal circumstances would be an acceptable change:

diff --git a/src/ipc.c b/src/ipc.c
index 36b3cb3..eefcaa3 100644
--- a/src/ipc.c
+++ b/src/ipc.c
@@ -165,9 +165,12 @@ static void check_detail(char *cmd, size_t len)
 {
    memset(cmd, 0, len);
    len = read(sd, cmd, len - 1);
-   if (len < 0)
-       return IPC_ERR;
-   if (len == 0)
-       return IPC_OK;
+   if (len == 0)
+       return IPC_OK;  
+   if (len < 0) {
+       errno = ENOTCONN; 
+       return IPC_ERR; 
+   }
+   
    cmd[len] = 0;

    for (size_t i = 0; i < NELEMS(cmds); i++) {

I've dropped all your comments. The reason/motivation for the code change goes in the commit message. The commit message should be limited to describe the actual change, not other issues also seen elsewhere. Comments in code should be limited to explaining non-intuitive code paths (this change is not it).

Dissecting the resulting code change and giving you a code review, I would say:

1. Changing the ordering of the if ()-statements that you've done here, has no logical benefit and there is no functional change.
2. Changing the error message from read() to ENOTCONN is not motivated, we really want to see the correct error code in the log message for the IPC_ERROR case in ipc_handle(). Changing the error code is also no functional change, i.e, does not change the execution of the IPC code.

Hence, I'm having a really hard time accepting that your change somehow fixes the problem you reported in issue #173. Now, instead of brutally closing this pull request, I'll give you the opportunity to respond and defend your code change, but bear in mind regardless of your motivation, I will not click the merge-button on this because honestly, this is one of the worst code changes I've ever seen.

[troglobit]

Closing as per discussion in #173.