Re: [PATCH] itd: use libhail's anet

2010-09-24 Thread Jeff Garzik

On 09/23/2010 08:35 PM, Jeff Garzik wrote:


Updated for libhail's anet.  Not yet committed, waiting for more time to
pass.

  Makefile.am  |2
  configure.ac |1
  iscsiutil.h  |   52 +
  target.c |   53 ++
  target.h |3
  util.c   |  223 
++-
  6 files changed, 57 insertions(+), 277 deletions(-)


This got pushed to itd.git late last night.

Jeff






--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

2010-09-24 Thread Jeff Garzik

On 09/24/2010 01:43 PM, Jim Meyering wrote:

Jeff Garzik wrote:

On 09/24/2010 07:32 AM, Jim Meyering wrote:

You can pull from the "oom" branch here:
git://git.infradead.org/users/meyering/tabled.git


Got nearly everything perfect.  Need one more minor yet important
change.  As described in doc/contributions.txt, every changeset MUST
have a Signed-off-by line at the end of a changeset's description.

I was able to pull and build just fine, so your git repo setup and
push appears correct.

Also, in your pull request, please put the branch immediately
following the repo URL on the same line, for easier cut-n-paste.
Here's how Linus requests his pull-requests to look:


Ok.  I've added those pesky S.O.B lines with this:

   git filter-branch --msg-filter \
 'cat&&  printf "\nSigned-off-by: Jim Meyering\n"' \
 HEAD~4..HEAD

and pushed the result.

Please pull from the 'oom' branch of
git://git.infradead.org/users/meyering/tabled.git


pulled from you & pushed upstream, thanks!


--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

2010-09-24 Thread Jim Meyering
Jeff Garzik wrote:
> On 09/24/2010 07:32 AM, Jim Meyering wrote:
>> You can pull from the "oom" branch here:
>>git://git.infradead.org/users/meyering/tabled.git
>
> Got nearly everything perfect.  Need one more minor yet important
> change.  As described in doc/contributions.txt, every changeset MUST
> have a Signed-off-by line at the end of a changeset's description.
>
> I was able to pull and build just fine, so your git repo setup and
> push appears correct.
>
> Also, in your pull request, please put the branch immediately
> following the repo URL on the same line, for easier cut-n-paste.
> Here's how Linus requests his pull-requests to look:

Ok.  I've added those pesky S.O.B lines with this:

  git filter-branch --msg-filter \
'cat && printf "\nSigned-off-by: Jim Meyering \n"' \
HEAD~4..HEAD

and pushed the result.

Please pull from the 'oom' branch of
git://git.infradead.org/users/meyering/tabled.git

I presume there's no need to re-post the diffstat or diffs.
--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

2010-09-24 Thread Jeff Garzik

On 09/24/2010 07:32 AM, Jim Meyering wrote:

You can pull from the "oom" branch here:
   git://git.infradead.org/users/meyering/tabled.git



Got nearly everything perfect.  Need one more minor yet important 
change.  As described in doc/contributions.txt, every changeset MUST 
have a Signed-off-by line at the end of a changeset's description.


I was able to pull and build just fine, so your git repo setup and push 
appears correct.


Also, in your pull request, please put the branch immediately following 
the repo URL on the same line, for easier cut-n-paste.  Here's how Linus 
requests his pull-requests to look:


---SNIP-
Please pull from 'upstream-linus' branch of
git://git.kernel.org/pub/scm/git/jgarzik/libata-dev.git upstream-linus

to receive the following updates:

 drivers/ata/ahci.c|  193 
+++--

 drivers/ata/libata-acpi.c |   40 +-
 drivers/ata/libata-core.c |3 +
 drivers/ata/libata.h  |2 +
 drivers/ata/pata_ali.c|2 +-
 include/linux/ata.h   |9 ++-
 include/linux/libata.h|   12 +++
 7 files changed, 178 insertions(+), 83 deletions(-)

Dirk Hohndel (1):
  pata_ali: trivial fix of a very frequent spelling mistake

Robert Hancock (1):
  ahci: display all AHCI 1.3 HBA capability flags (v2)

Tejun Heo (5):
  ahci: disable 64bit DMA by default on SB600s
  libata: cosmetic updates
  libata: implement more acpi filtering options
  libata: make gtf_filter per-dev
  ahci: filter FPDMA non-zero offset enable for Aspire 3810T

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index acd1162..4edca6e 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
[COMBINED PATCH FOLLOWS...]

---SNIP-
--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

2010-09-24 Thread Jim Meyering
Jim Meyering wrote:
> Jeff Garzik wrote:
>
>> On 09/23/2010 03:19 PM, Jeff Garzik wrote:
>>> 3) I process patches similar to how Linus and others in the kernel do
>>> it: "git am /path/to/mbox_of_patches" That tends to impose some
>>> restrictions on the contents of each email.
>>
>> FWIW, 'git pull' submissions are welcome.  Standard kernel-style pull
>> submission style applies[1].
>>
>>  Jeff
>>
>> [1] public git pull URL including branch name, diffstat, shortlog or
>> full log of changeset summaries, and finally, the combined diff of all
>> changes.
>
> Here you go.
> You can pull from the "oom" branch here:
>   git://git.infradead.org/users/meyering/tabled.git
>
> I think I've addressed all of your preferences, merging
> most OOM fixes into one commit, but not the two you mentioned
> that should stay separate.  I also left the sizeof(s) one separate.
>
> However, I did leave the copyright year updates in.
> If they're a problem, let me know and I'll do another round.
> Otherwise, I can send a patch to update all of the
> remaining ones to include 2010 so this won't be an
> issue for 3 more months.
>
> $ git shortlog HEAD ^origin/master
> Jim Meyering (4):
>   server/server.c: use sizeof(s) rather than equivalent "64"
>   don't dereference NULL on OOM
>   server/status.c: don't deref NULL on failed strdup
>   server/bucket.c: don't deref NULL upon failed malloc
>
>  b/server/bucket.c  |   25 -
>  b/server/config.c  |7 +--
>  b/server/object.c  |7 ++-
>  b/server/replica.c |7 +--
>  b/server/server.c  |2 +-
>  b/server/status.c  |8 +---
>  server/server.c|   13 +
>  7 files changed, 47 insertions(+), 22 deletions(-)

That wasn't quite right.
Here's an updated patch, including e.g., this:

diff --git a/server/config.c b/server/config.c
index a58a0e6..9539cfd 100644
--- a/server/config.c
+++ b/server/config.c
@@ -436,7 +436,8 @@ void read_config(void)

memset(&ctx, 0, sizeof(struct config_context));

-   if (!(tabled_srv.port = strdup("8080"))) {
+   tabled_srv.port = strdup("8080");
+   if (!tabled_srv.port) {
applog(LOG_ERR, "no core");
exit(1);
}

==
diff --git a/server/bucket.c b/server/bucket.c
index eb03e03..cf42d2d 100644
--- a/server/bucket.c
+++ b/server/bucket.c
@@ -1,6 +1,6 @@

 /*
- * Copyright 2008-2009 Red Hat, Inc.
+ * Copyright 2008-2010 Red Hat, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -788,29 +788,36 @@ static GList *bucket_list_pfx(GList *content, GHashTable 
*common_pfx,
s = malloc(cpfx_len);
p = s;

+#define append_const(buf, c) \
+  do { memcpy(buf, c, sizeof(c)-1); (buf) += sizeof(c)-1; } while (0)
+
tmpl = pfx_list;
while (tmpl) {
prefix = (char *) tmpl->data;
pfx_len = strlen(prefix);

-   memcpy(p, optag, sizeof(optag)-1);  p += sizeof(optag)-1;
-   memcpy(p, pfoptag, sizeof(pfoptag)-1);  p += sizeof(pfoptag)-1;
-   memcpy(p, prefix, pfx_len);  p += pfx_len;
-   memcpy(p, delim, delim_len);  p += delim_len;
-   memcpy(p, pfedtag, sizeof(pfedtag)-1);  p += sizeof(pfedtag)-1;
-   memcpy(p, edtag, sizeof(edtag)-1);  p += sizeof(edtag)-1;
+   if (p) {
+   append_const(p, optag);
+   append_const(p, pfoptag);
+   memcpy(p, prefix, pfx_len);  p += pfx_len;
+   memcpy(p, delim, delim_len);  p += delim_len;
+   append_const(p, pfedtag);
+   append_const(p, edtag);
+   }

free(prefix);

tmpl = tmpl->next;
}
-   *p = 0;
+   if (p)
+   *p = 0;

free(delim);
g_list_free(pfx_list);

-   return g_list_append(content, s);
+   return s ? g_list_append(content, s) : content;
 }
+#undef append_const

 struct bucket_list_info {
char *prefix;
diff --git a/server/config.c b/server/config.c
index f94886e..dad6579 100644
--- a/server/config.c
+++ b/server/config.c
@@ -1,6 +1,6 @@

 /*
- * Copyright 2009 Red Hat, Inc.
+ * Copyright 2009, 2010 Red Hat, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -437,6 +437,10 @@ void read_config(void)
memset(&ctx, 0, sizeof(struct config_context));

tabled_srv.port = strdup("8080");
+   if (!tabled_srv.port) {
+   applog(LOG_ERR, "no core");
+   exit(1);
+   }

if (!g_file_get_contents(tabled_srv.config, &text, &len, NULL)) {
applog(LOG_ERR, "failed to read config file %s",
diff --git a/server/ob

Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

2010-09-24 Thread Jim Meyering
Jeff Garzik wrote:

> On 09/23/2010 03:19 PM, Jeff Garzik wrote:
>> 3) I process patches similar to how Linus and others in the kernel do
>> it: "git am /path/to/mbox_of_patches" That tends to impose some
>> restrictions on the contents of each email.
>
> FWIW, 'git pull' submissions are welcome.  Standard kernel-style pull
> submission style applies[1].
>
>   Jeff
>
> [1] public git pull URL including branch name, diffstat, shortlog or
> full log of changeset summaries, and finally, the combined diff of all
> changes.

Here you go.
You can pull from the "oom" branch here:
  git://git.infradead.org/users/meyering/tabled.git

I think I've addressed all of your preferences, merging
most OOM fixes into one commit, but not the two you mentioned
that should stay separate.  I also left the sizeof(s) one separate.

However, I did leave the copyright year updates in.
If they're a problem, let me know and I'll do another round.
Otherwise, I can send a patch to update all of the
remaining ones to include 2010 so this won't be an
issue for 3 more months.

$ git shortlog HEAD ^origin/master
Jim Meyering (4):
  server/server.c: use sizeof(s) rather than equivalent "64"
  don't dereference NULL on OOM
  server/status.c: don't deref NULL on failed strdup
  server/bucket.c: don't deref NULL upon failed malloc

 b/server/bucket.c  |   25 -
 b/server/config.c  |7 +--
 b/server/object.c  |7 ++-
 b/server/replica.c |7 +--
 b/server/server.c  |2 +-
 b/server/status.c  |8 +---
 server/server.c|   13 +
 7 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/server/bucket.c b/server/bucket.c
index eb03e03..cf42d2d 100644
--- a/server/bucket.c
+++ b/server/bucket.c
@@ -1,6 +1,6 @@

 /*
- * Copyright 2008-2009 Red Hat, Inc.
+ * Copyright 2008-2010 Red Hat, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -788,29 +788,36 @@ static GList *bucket_list_pfx(GList *content, GHashTable 
*common_pfx,
s = malloc(cpfx_len);
p = s;

+#define append_const(buf, c) \
+  do { memcpy(buf, c, sizeof(c)-1); (buf) += sizeof(c)-1; } while (0)
+
tmpl = pfx_list;
while (tmpl) {
prefix = (char *) tmpl->data;
pfx_len = strlen(prefix);

-   memcpy(p, optag, sizeof(optag)-1);  p += sizeof(optag)-1;
-   memcpy(p, pfoptag, sizeof(pfoptag)-1);  p += sizeof(pfoptag)-1;
-   memcpy(p, prefix, pfx_len);  p += pfx_len;
-   memcpy(p, delim, delim_len);  p += delim_len;
-   memcpy(p, pfedtag, sizeof(pfedtag)-1);  p += sizeof(pfedtag)-1;
-   memcpy(p, edtag, sizeof(edtag)-1);  p += sizeof(edtag)-1;
+   if (p) {
+   append_const(p, optag);
+   append_const(p, pfoptag);
+   memcpy(p, prefix, pfx_len);  p += pfx_len;
+   memcpy(p, delim, delim_len);  p += delim_len;
+   append_const(p, pfedtag);
+   append_const(p, edtag);
+   }

free(prefix);

tmpl = tmpl->next;
}
-   *p = 0;
+   if (p)
+   *p = 0;

free(delim);
g_list_free(pfx_list);

-   return g_list_append(content, s);
+   return s ? g_list_append(content, s) : content;
 }
+#undef append_const

 struct bucket_list_info {
char *prefix;
diff --git a/server/config.c b/server/config.c
index f94886e..a58a0e6 100644
--- a/server/config.c
+++ b/server/config.c
@@ -1,6 +1,6 @@

 /*
- * Copyright 2009 Red Hat, Inc.
+ * Copyright 2009, 2010 Red Hat, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -436,7 +436,10 @@ void read_config(void)

memset(&ctx, 0, sizeof(struct config_context));

-   tabled_srv.port = strdup("8080");
+   if (!(tabled_srv.port = strdup("8080"))) {
+   applog(LOG_ERR, "no core");
+   exit(1);
+   }

if (!g_file_get_contents(tabled_srv.config, &text, &len, NULL)) {
applog(LOG_ERR, "failed to read config file %s",
diff --git a/server/object.c b/server/object.c
index 3801e94..1f2f68f 100644
--- a/server/object.c
+++ b/server/object.c
@@ -1,6 +1,6 @@

 /*
- * Copyright 2008-2009 Red Hat, Inc.
+ * Copyright 2008-2010 Red Hat, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -801,6 +801,11 @@ static bool object_put_body(struct client *cli, const char 
*user,
cli->out_objid = objid;
cli->out_user = strdup(user);

+   if (!cli->out_bucket || !cli->out_key || !cli->out_user) {
+   applog(LOG_ERR, "OOM in object_put_body");
+ 

Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

2010-09-24 Thread Jim Meyering
Jeff Garzik wrote:
> On 09/23/2010 04:43 AM, Jim Meyering wrote:
>>> From fb7865d158b0d32907dde703c4d37c70a26e738c Mon Sep 17 00:00:00 2001
>> From: Jim Meyering
>> Date: Thu, 23 Sep 2010 10:11:44 +0200
>> Subject: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM
>
> (see other email for general response to these changes, comments on
> GLib, OOM, etc.)
>
> First off, I ACK (accept) all these changes.  Technically they appear
> correct, and I am interested in merging them.
>
> But I request a few minor style and workflow adjustments, and a
> resubmission. Specific comments:
>
> [style]
>
> 1) the functional style of sizeof keyword, with parens, is preferred:
>
> - snprintf(s, 64, "get user '%s'", user);
> + snprintf(s, sizeof s, "get user '%s'", user);

Sure.  Adjusted.

> 2) it is preferred to omit optional braces for singleton test+stmt
> style statements:
>
> + if (!pass) {
> + goto err_cmp;
> + }

Gladly.  That's what I would have done in code I own, but there is a
braced single-line "else" block just above, so I presumed that the
style was "always use braces".  (I think we have the same preferences,
since I too would use braces around the single-line "else" in that case,
though not if the "then" block had also been a one-liner.

> [patch submission administrivia]
>
> 3) I process patches similar to how Linus and others in the kernel do
> it: "git am /path/to/mbox_of_patches"  That tends to impose some
> restrictions on the contents of each email.
>
> In your case, while the patch descriptions and diffs themselves are
> correct, you seem to be sending one-mbox-per-email, while I'm
> expecting one-patch-per-email.  If you could tweak your process to
> make that change, that would reduce the manual labor on my part.

No problem.

> 4) While total number of patches is not really a problem, I would
> request sweeping most of the one-and-two-liners in this series into a
> single patch, leaving perhaps only the bucket.c and status.c changes
> as standalone patches.

Will do.
You can tell that I'm too accustomed to posting FYI-patches
that I will shortly push -- or that I'll push upon review.

> It's more an art and style preferences, than science, when deciding
> how to separate out changes into patches. Trying to take my cues from
> the kernel, it is preferred, for example, that bug fixes be separate
> from new features, or whitespace and cosmetic changes separate from
> functional changes.  But it is also encouraged to group similar
> changes together, if, for example, you're making a similar change
> across a large number of files.
>
> Mailing list review-ability, useful 'git bisect' boundaries, and a
> coherent 'git shortlog' summary tend to be my guides when deciding
> patch boundaries.

Preaching to the choir ;-)

Thanks for spelling out your guidelines.
--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html