Skip to Content.
Sympa Menu

ndt-dev - Re: [ndt-dev] Submitting a patch

Subject: NDT-DEV email list created

List archive

Re: [ndt-dev] Submitting a patch


Chronological Thread 
  • From: Kavitha Kumar <>
  • To: Dominic Hamon <>
  • Cc:
  • Subject: Re: [ndt-dev] Submitting a patch
  • Date: Thu, 11 Oct 2012 14:35:40 -0400

Hi Dominic,

      Thanks ! Most of your changes look good. 

A few places where there are questions are:

 configure.ac
--------------------
Referring to line : "CFLAGS="-pedantic -Wall -Werror -Wno-unused-result -fno-strict-aliasing -std=gnu99"
1. The -Wno-unused-result option is not available with CentOS 5. So, this cannot be included in NDT right now.
2.  It may be better to skip the -Werror for now. We may be able to include it in later versions after we have had a chance to test it with more environments. Were you just including it to help with removing warnings? Also, with a few other libraries (that may not have been installed on your test/build system), I saw "errors" which were actually warnings, but being interpreted as "errors" with this option.
3. We may also want to skip the -fno-strict aliasing for now.

logging.c
--------------
Referring to changes in char *get_currenttime(char *isoTime, int isotimearrsize) :
Can you please explain your intention in doing this? We are tending towards not having this change for now.

I built/installed the server+client with just the "-std=99" CFLAG option, and the results looked comparable to the previous version. (Some files you changed are included in the server side too). I did not concentrate
on IPv6 option testing (your changes in troute6.c), but recall MLAB being involved in some work relating to that. It would be great if you verified that section.

Thanks,
Kavitha





On Oct 9, 2012, at 5:44 PM, Dominic Hamon wrote:

Hi Kavitha

As requested, I have attached the patch. This fixes a few subtle issues
including out-of-bounds array accesses and invalid pointer/int
conversion and occasions where the wrong type was being passed 

methods.

I've tested it by pointing the client at a running known-good server.

Thanks
- dominic

On Mon, Oct 08, 2012 at 04:46:10PM -0400, Kavitha Kumar wrote:
Hi Dominic,

 Pls send the patches to this list for review. Folks here on ndt-dev can review and/or apply them. We can then decide further steps to committing the changes.

Do you foresee contributing to NDT in a bigger fashion soon/in future and thus like commit access?

thanks,
Kavitha

----- Original Message -----
From:
To:
Sent: Monday, October 8, 2012 3:29:16 PM
Subject: [ndt-dev] Submitting a patch

Hello

I have prepared a patch that fixes a couple of issues with trunk.
Mostly
innocuous changes to clean up C90 warnings but some of the changes
fix things
like implicit int/pointer conversion.

How do I go about submitting the patch for review?

Thanks
- dominic

<fix-warnings.patch>




Archive powered by MHonArc 2.6.16.

Top of Page