headers
Matthias Bolte | 1 Sep 2010 17:32

Re: [libvirt] [PATCH] esx: Fix generator for string return values

2010/8/30 Eric Blake <eblake <at> redhat.com>:
> On 08/29/2010 05:00 PM, Matthias Bolte wrote:
>>
>> Distinguish between strings as parameters (const char *)
>> and strings as return values (char **).
>
> Here, you mention char**,
>
>>          if self.type == "String" and \
>>             self.occurrence not in [OCCURRENCE__REQUIRED_LIST,
>>                                     OCCURRENCE__OPTIONAL_LIST]:
>> -            return "const char *"
>> +            if self.is_return_value:
>> +                return "char *"
>
> But here, it is only char*.  Which is it?

That's a bit tricky. In case Parameter.get_type_string() is called
from Parameter.generate_return() then

  string += "%s*%s)%s" % (self.get_type_string(), self.name, end_of_line)

in line 104 adds the second *.

I attached v2 where this is simplified.

Matthias
Attachment (0001-esx-Fix-generator-for-string-return-values.diff): text/x-diff, 6778 bytes
(Continue reading)






Permalink
| Reply
| 2 comments  
| 



  

  

  





headers


Daniel Veillard
 | 
1 Sep 2010 18:07






Picon

Favicon

Gravatar





[libvirt] Next release 0.8.4 proposed schedule


 So it's already September and while back from vacations I didn't yet
fully scan all the list for potential patches. Still I'm tempted to try
to get a release by the 10 which mean we would enter the freeze this
week-end. I guess it would be 0.8.4 since there is no huge feature or
change commited since 0.8.3.
  Is there anything important shich should land before the next release
and that I may have overlooked ?
  There is also a number of patches which were sent but not ACK'ed or
pushed but since I didn't finished the full list, I don't know yet if
they got reposted, I will try to finish the scan tomorrow. But if you
have patches blocked missing review please raise them !

  thanks,

Daniel

--

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel <at> veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/







Permalink
| Reply
| 0 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 18:21






Picon

Favicon

Gravatar





Re: [libvirt] [PATCH] esx: Rework datastore path parsing and	handling


On 08/26/2010 04:37 PM, Matthias Bolte wrote:
> Instead of splitting the path part of a datastore path into
> directory and file name, keep this in one piece. An example:
>
>    "[datastore] directory/file"
>
> was split into this before:
>
>    datastoreName = "datastore"
>    directoryName = "directory"
>    fileName = "file"
>
> Now it's split into this:
>
>    datastoreName = "datastore"
>    directoryName = "directory"
>    directoryAndFileName = "directory/file"

Seems reasonable.

> This simplifies code using esxUtil_ParseDatastorePath, because
> directoryAndFileName is used more often than fileName. Also the
> old approach expected the datastore path to reference a actual

s/ a / an /

>  <at>  <at>  -52,8 +52,7  <at>  <at>  typedef struct _esxVMX_Data esxVMX_Data;
>
>   struct _esxVMX_Data {
>       esxVI_Context *ctx;

(Continue reading)






Permalink
| Reply
| 3 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 18:30






Picon

Favicon

Gravatar





Re: [libvirt] [PATCH] esx: Fix generator for string return values


On 09/01/2010 09:32 AM, Matthias Bolte wrote:
> That's a bit tricky. In case Parameter.get_type_string() is called
> from Parameter.generate_return() then
>
>    string += "%s*%s)%s" % (self.get_type_string(), self.name, end_of_line)
>
> in line 104 adds the second *.
>
> I attached v2 where this is simplified.

Yes, it's much easier to see in v2.  ACK, if you trust my minimal python 
reviewing skills.

--

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org







Permalink
| Reply
| 1 comment  
| 



  

  

  





headers


Cole Robinson
 | 
1 Sep 2010 21:43






Picon

Favicon

Gravatar





[libvirt] [PATCH] buf: Fix possible infinite loop in EscapeString,	VSnprintf


The current code will go into an infinite loop if the printf generated
string is >= 1000, AND exactly 1 character smaller than the amount of free
space in the buffer. When this happens, we are dropped into the loop body,
but nothing will actually change, because count == (buf->size - buf->use - 1),
and virBufferGrow returns unchanged if count < (buf->size - buf->use)

Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle
the NULL byte for us anyways, so we shouldn't need to manually accomodate
for it.

Here's a bug where we are actually hitting this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=602772

Signed-off-by: Cole Robinson <crobinso <at> redhat.com>
---
 src/util/buf.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/util/buf.c b/src/util/buf.c
index fc1217b..734b8f6 100644
--- a/src/util/buf.c
+++ b/src/util/buf.c
 <at>  <at>  -236,11 +236,11  <at>  <at>  virBufferVSprintf(const virBufferPtr buf, const char *format, ...)
         virBufferGrow(buf, 100) < 0)
         return;

-    size = buf->size - buf->use - 1;
+    size = buf->size - buf->use;
     va_start(argptr, format);
     va_copy(locarg, argptr);

(Continue reading)






Permalink
| Reply
| 2 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 21:59






Picon

Favicon

Gravatar





[libvirt] [PATCH 0/7] round 2 of sprintf cleanups


I don't think all snprintf uses are bad - those that format a number
into an appropriately-sized maximum-possible array should be fine.
For example, there are some fixed-size buffers whose size is determined
by the kernel, where snprintf was the appropriate course of action
in nwfilter_ebiptables_driver.c.  But sprintf doesn't have quite the
coverage as snprintf, so this continuation of *printf cleanups converts
some of these.

I still have more patches to go, but this has taken me long enough
to get some more review rather than slamming a 20-patch series all
at the end.

More comments within individual patches.

Eric Blake (7):
  build: add some modules
  vbox: factor a large function
  vbox: avoid problematic uses of sprintf
  network: use virAsprintf when appropriate
  lxc: avoid large stacks with veth creation
  openvz: formatting cleanups
  openvz: use virAsprintf to avoid large stacks

 bootstrap.conf             |    2 +
 src/conf/network_conf.c    |   15 +-
 src/lxc/lxc_driver.c       |   32 +-
 src/lxc/veth.c             |   84 ++-
 src/lxc/veth.h             |    6 +-
 src/openvz/openvz_conf.c   |  142 ++--
 src/openvz/openvz_driver.c |   23 +-

(Continue reading)






Permalink
| Reply
| 21 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 21:59






Picon

Favicon

Gravatar





[libvirt] [PATCH 1/7] build: add some modules


snprintf is currently implicitly picked up by getaddrinfo, but we
might as well make it explicit so that mingw doesn't break if
getaddrinfo changes to drop the dependency.

func doesn't matter for gcc compilation, but may help other compilers
cope with our use of __func__.

* bootstrap.conf (gnulib_modules): Add snprintf and func.
---

Shouldn't impact a GNU/Linux build.

 bootstrap.conf |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index ca31a6e..8a85b91 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
 <at>  <at>  -29,6 +29,7  <at>  <at>  count-one-bits
 crypto/md5
 dirname-lgpl
 fcntl-h
+func
 getaddrinfo
 gethostname
 getpass
 <at>  <at>  -53,6 +54,7  <at>  <at>  random_r
 sched
 send

(Continue reading)






Permalink
| Reply
| 2 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 21:59






Picon

Favicon

Gravatar





[libvirt] [PATCH 6/7] openvz: formatting cleanups


* src/openvz/openvz_conf.c: Whitespace fixes.
* src/openvz/openvz_driver.c: Likewise.
---

Should just be formatting, no content change.

openvz has other problems, like its use of popen (totally unsafe);
so I'll be fixing it some more when I get to the virCommand patch
series.

 src/openvz/openvz_conf.c   |   70 ++++++++++++++++++++++----------------------
 src/openvz/openvz_driver.c |   23 +++++++-------
 2 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index b52f4ac..356c7f0 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
 <at>  <at>  -64,7 +64,7  <at>  <at>  strtoI(const char *str)
     int val;

     if (virStrToLong_i(str, NULL, 10, &val) < 0)
-        return 0 ;
+        return 0;

     return val;
 }
 <at>  <at>  -338,7 +338,7  <at>  <at>  openvz_replace(const char* str,
     from_len = strlen(from);
     to_len = strlen(to);

(Continue reading)






Permalink
| Reply
| 2 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 21:59






Picon

Favicon

Gravatar





[libvirt] [PATCH 3/7] vbox: avoid problematic uses of sprintf


* src/vbox/vbox_tmpl.c (vboxStartMachine, vboxAttachUSB): Use
virAsprintf instead.
---

This removes all use of sprintf in vbox.  The first 3 use virAsprintf
(DISPLAY may be arbitrarily long, and while we are unlikely to hit
9999 devices, it's better to be safe than to risk silent buffer
overflow); the remaining two are sized appropriately (actually, they
are sized too large, the real boundary size would be sizeof(int)*2+1
rather than 40); I felt better using snprintf rather than sprintf.

This doesn't address the fact that vbox doesn't really have very good
OOM handling (ie. it keeps on trying, although after the first OOM,
it will likely get another one); but that is an independent issue.

 src/vbox/vbox_tmpl.c |   39 +++++++++++++++++++++++++++------------
 1 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3e8ff23..f50a12e 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
 <at>  <at>  -3161,7 +3161,6  <at>  <at>  vboxStartMachine(virDomainPtr dom, int i, IMachine *machine, vboxIID *iid)
     PRUnichar *valueDisplayUtf16 = NULL;
     char      *valueDisplayUtf8  = NULL;
     IProgress *progress          = NULL;
-    char displayutf8[32]         = {0};
     PRUnichar *env               = NULL;
     PRUnichar *sessionType       = NULL;
     nsresult rc;

(Continue reading)






Permalink
| Reply
| 2 comments  
| 



  

  

  





headers


Eric Blake
 | 
1 Sep 2010 21:59






Picon

Favicon

Gravatar





[libvirt] [PATCH 5/7] lxc: avoid large stacks with veth creation


* src/lxc/veth.h (vethCreate): Change prototype.
* src/lxc/veth.c (vethCreate): Always malloc veth2, and allocate
veth1 if needed.
(getFreeVethName): Adjust signature, and use virAsprintf.
* src/lxc/lxc_driver.c (lxcSetupInterfaces): Adjust caller.
---

This issue crossed file boundaries.  It was a bit tricky, since
vethCreate was used in two patterns - one where the parent name was
already known, and another where the parent name is picked as the
first available option.  But the end result avoids strdup'ing a
fixed-width buffer, and I think I correctly avoided any leaks (in
lxcSetupInterfaces, once a string is transferred to *veths, then
returning failure will cause that string to be freed in the caller).
It also gets rid of a PATH_MAX stack over-allocation.

 src/lxc/lxc_driver.c |   32 ++++++++-----------
 src/lxc/veth.c       |   84 ++++++++++++++++++++++++++++++-------------------
 src/lxc/veth.h       |    6 ++--
 3 files changed, 67 insertions(+), 55 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6fe20b1..326fee6 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
 <at>  <at>  -812,14 +812,16  <at>  <at>  static int lxcSetupInterfaces(virConnectPtr conn,
         return -1;

     for (i = 0 ; i < def->nnets ; i++) {
-        char parentVeth[PATH_MAX] = "";

(Continue reading)






Permalink
| Reply
| 2 comments  
| 



  

  

  

  


  





    



Gmane