[PATCH 0/5] CERN - Proposed distcc Modifications.

[Ian....@cern.ch]

As requested here is the output from the interidff command
for the patches, and a link to the previous review discussion.

Thanks.

Ian

[Ian....@cern.ch]

[Ian....@cern.ch]

[Ian....@cern.ch]

[Ian....@cern.ch]

[Fergus Henderson]

Hi Ian,

On Mon, May 11, 2009 at 10:16 AM, <Ian....@cern.ch> wrote:

Link to previous review discussion:

http://groups.google.com/group/distcc-patches/browse_thread/thread/782679a5067c867c#

For future reference, it really makes these discussions a lot easier to follow if you reply on the same thread, rather than composing a new email every time.
Now I have the relative diff, and a link to the previous review discussion, but I don't have the full diff in the same thread.
 

--- quilt-distcc-3.1/distcc-3.1/src/auth_common.c
+++ quilt-distcc-3.1/distcc-3.1/src/auth_common.c
diff -u quilt-distcc-3.1/distcc-3.1/src/auth_distcc.c quilt-distcc-3.1/distcc-3.1/src/auth_distcc.c
--- quilt-distcc-3.1/distcc-3.1/src/auth_distcc.c
+++ quilt-distcc-3.1/distcc-3.1/src/auth_distcc.c
@@ -35,7 +35,6 @@
 #include "netutil.h"
 #include "trace.h"

-static int dcc_gssapi_get_services_from_env_and_build_flags(OM_uint32 *req_flags);
 static int dcc_gssapi_establish_secure_context(int to_net_sd,
                                              int from_net_sd,
                                              OM_uint32 req_flags,
@@ -49,7 +48,7 @@

 /*
 * Perform any requested security.  Message replay and out of sequence
- * detection are given in adition to those specified by the user.
+ * detection are given in adition to mutual authentication.

"adition" should be "addition".

Also, I think it would probably be better to use ",auth" rather than ",gssapi".
"gssapi" is going to be pretty obscure to most users, I think.

Otherwise, that looks fine.

--
Fergus Henderson <fergus.h...@gmail.com>

[Fergus Henderson]

Thanks for the patches.
I haven't forgotten about these, but I have been busy with some other tasks this week.
I'll try to get to them sometime in the next week.

On Mon, May 11, 2009 at 10:16 AM, <Ian....@cern.ch> wrote:

--
Fergus Henderson <fer...@google.com>

[Fergus Henderson]

Hi,

Thanks for the patch and sorry for the very slow review.

On Mon, May 11, 2009 at 10:16 AM, <Ian....@cern.ch> wrote:

Link to previous review discussion:

http://groups.google.com/group/distcc-patches/browse_thread/thread/b3e078dd8a9adf81#

This patch was ready to be committed, but the resolution has been
increased to include microseconds.


diff -u quilt-distcc-3.1/distcc-3.1/src/trace.c quilt-distcc-3.1/distcc-3.1/src/trace.c
--- quilt-distcc-3.1/distcc-3.1/src/trace.c
+++ quilt-distcc-3.1/distcc-3.1/src/trace.c
@@ -39,6 +39,7 @@
 #include <stdarg.h>
 #include <syslog.h>
 #include <time.h>
+#include <sys/time.h>

 #include "trace.h"
 #include "snprintf.h"
@@ -322,23 +323,32 @@
    size_t len;
    char timebuf[128];
    size_t timebuflen;
-    time_t now;
    struct tm *timestamp;
    char *timestamp_opt;
+    char *entry = NULL;
+    struct timeval time_val;
+    time_t curr_time;

    if ((timestamp_opt = getenv("DISTCC_TIMESTAMP"))) {
-       if (*timestamp_opt == '1') {
-            now = time(NULL);
+        if (*timestamp_opt == '1') {
+            gettimeofday(&time_val, NULL);
+            curr_time = time_val.tv_sec;

-           if ((timestamp = localtime(&now)) != NULL) {
-               timebuflen = strftime(timebuf, 128, "%F %T ", timestamp);
-           } else {
-                strcpy(timebuf, "localtime() NULL??? ");
-               timebuflen = strlen(timebuf);
-           }
+            if ((timestamp = localtime(&curr_time)) != NULL) {
+                timebuflen = strftime(timebuf, 128, "%F %T", timestamp);
+
+                if (asprintf(&entry, "%s.%ld ", timebuf, time_val.tv_usec) < 0) {

Here you call asprintf(), which allocates memory with malloc(), but there's no corresponding call to free(), so this will result in a memory leak.  That needs to be fixed before this can be committed.
 

If you post a revised version of this patch, please also include "interdiff" output showing the changes from the previous patch, and if possible please post in the same thread as this post (e.g. using the "reply" command in your email reader, rather than composing a new email), or if not, please include a link to this thread.

Cheers,
  Fergus.
--
Fergus Henderson <fer...@google.com>

[Fergus Henderson]

I have applied this patch, after making the two changes that I suggested below.

--
Fergus Henderson <fergus.h...@gmail.com>