| Message ID | 1402673533-13243-2-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On Fri, Jun 13, 2014 at 05:32:10PM +0200, Aurelien Jarno wrote: > The current comments concerning nserv and nservall are not really clear > and lead to confusion when reviewing an already complex code. Improve > them, there real meaning have been confirmed by a code analysis. > > 2014-06-13 Aurelien Jarno <aurelien@aurel32.net> > > * resolv/res_init.c (__res_vinit): Improve comments > about nserv and nservall. I already acked this. Siddhesh > > diff --git a/resolv/res_init.c b/resolv/res_init.c > index ea133f8..bdec4d9 100644 > --- a/resolv/res_init.c > +++ b/resolv/res_init.c > @@ -153,9 +153,9 @@ __res_vinit(res_state statp, int preinit) { > char *cp, **pp; > int n; > char buf[BUFSIZ]; > - int nserv = 0; /* number of nameserver records read from file */ > + int nserv = 0; /* number of IPv4 nameservers read from file */ > #ifdef _LIBC > - int nservall = 0; /* number of NS records read, nserv IPv4 only */ > + int nservall = 0; /* number of (IPv4 + IPV6) nameservers read from file */ > #endif > int haveenv = 0; > int havesearch = 0; > -- > 2.0.0 >
On Wed, Jun 18, 2014 at 02:42:25PM +0530, Siddhesh Poyarekar wrote: > On Fri, Jun 13, 2014 at 05:32:10PM +0200, Aurelien Jarno wrote: > > The current comments concerning nserv and nservall are not really clear > > and lead to confusion when reviewing an already complex code. Improve > > them, there real meaning have been confirmed by a code analysis. > > > > 2014-06-13 Aurelien Jarno <aurelien@aurel32.net> > > > > * resolv/res_init.c (__res_vinit): Improve comments > > about nserv and nservall. > > I already acked this. > Thanks for your ack. My plan is to commit the 4 patches at the same time, as they are linked. I am waiting for the review of the 3 other patches, unless I am mistaken and that this ack is for the whole series.
On Wed, Jun 18, 2014 at 06:28:31PM +0200, Aurelien Jarno wrote: > Thanks for your ack. My plan is to commit the 4 patches at the same > time, as they are linked. I am waiting for the review of the 3 other > patches, unless I am mistaken and that this ack is for the whole series. They're linked, but not dependent, but it's your personal preference I guess. My concern is that it breaks the patchwork workflow, since you're resent the acked patch again. I haven't looked at the other patches to see if they're changed, but if they're not and your intention was to just ping the patches, then I'd suggest using the method everyone else is using, which is to add [ping] to the subject line in reply to your original submission instead of resending the patch, since the latter adds an additional patch to review in patchwork. That, or help maintain the patchwork reviews, and I'd really be glad if you choose the latter :) Siddhesh
On Thu, Jun 19, 2014 at 03:21:42PM +0530, Siddhesh Poyarekar wrote: > On Wed, Jun 18, 2014 at 06:28:31PM +0200, Aurelien Jarno wrote: > > Thanks for your ack. My plan is to commit the 4 patches at the same > > time, as they are linked. I am waiting for the review of the 3 other > > patches, unless I am mistaken and that this ack is for the whole series. > > They're linked, but not dependent, but it's your personal preference I > guess. My concern is that it breaks the patchwork workflow, since > you're resent the acked patch again. I haven't looked at the other > patches to see if they're changed, but if they're not and your > intention was to just ping the patches, then I'd suggest using the > method everyone else is using, which is to add [ping] to the subject > line in reply to your original submission instead of resending the > patch, since the latter adds an additional patch to review in > patchwork. One of the patch has changed, all the others got improved description and a changelog entry. But more importantly the first version was clearly an RFC, one of the main change between the two is that I am now convinced it's the right thing to do. > That, or help maintain the patchwork reviews, and I'd really be glad > if you choose the latter :) Yes, in that case I could simply flag the patch as superseded in patchworks. I have just created an account, so that I can do that next time.
On Thu, Jun 19, 2014 at 05:53:37PM +0200, Aurelien Jarno wrote: > Yes, in that case I could simply flag the patch as superseded in > patchworks. I have just created an account, so that I can do that next > time. ... and I have added you as a maintainer so that you can change state of patches you post and review. Thanks! Siddhesh
On Thu, Jun 19, 2014 at 9:13 AM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote: > On Thu, Jun 19, 2014 at 05:53:37PM +0200, Aurelien Jarno wrote: >> Yes, in that case I could simply flag the patch as superseded in >> patchworks. I have just created an account, so that I can do that next >> time. > > ... and I have added you as a maintainer so that you can change state > of patches you post and review. > I checked it in.
diff --git a/resolv/res_init.c b/resolv/res_init.c index ea133f8..bdec4d9 100644 --- a/resolv/res_init.c +++ b/resolv/res_init.c @@ -153,9 +153,9 @@ __res_vinit(res_state statp, int preinit) { char *cp, **pp; int n; char buf[BUFSIZ]; - int nserv = 0; /* number of nameserver records read from file */ + int nserv = 0; /* number of IPv4 nameservers read from file */ #ifdef _LIBC - int nservall = 0; /* number of NS records read, nserv IPv4 only */ + int nservall = 0; /* number of (IPv4 + IPV6) nameservers read from file */ #endif int haveenv = 0; int havesearch = 0;