Libguestfs - Jun 2015 - Re: [PATCH 2/3] inspection: Add support for CoreOS

Sorry, I don't get it. In inspect.c line 67 you have this:

  for (fs = fses; *fs; fs += 2) {
    if (guestfs_int_check_for_filesystem_on (g, *fs)) {
      guestfs_int_free_inspect_info (g);
      return NULL;
    }
  }

I don't see the bug.

On 02/06/15 18:30, Richard W.M. Jones wrote:
> On Tue, Jun 02, 2015 at 06:18:38PM +0300, Nikos Skalkotos wrote:
>> On 02/06/15 17:10, Richard W.M. Jones wrote:
>> Hello,
>>
>>> On Fri, May 29, 2015 at 12:24:58PM +0300, Nikos Skalkotos wrote:
>>>> +  if (collect_coreos_inspection_info (g)) {
>>>> +    guestfs_int_free_inspect_info (g);
>>>> +    return NULL;
>>>> +  }
>>> Although this is stylistic, I think it's easier to understand
if
>>> you change the if condition to:
>>>
>>>   if (collect_coreos_inspection_info (g) == -1) {
>>>     ...
>>>
>> Since we 'll be using safe_realloc, I don't think there is a
need for
>> any of:
>>  * guestfs_int_merge_fs_inspections()
>>  * collect_coreos_inspection_info()
>> to be returning a value at all. So, I'll remove the if check
completely.
>>
>> Nikos
>>
>> P.S. You have a check like this one on
>> guestfs_int_check_for_filesystem_on() a few lines above :-)
> In extend_fses?  That's a bug ..
>
> Rich.
>

On Tue, Jun 02, 2015 at 06:37:06PM +0300, Nikos Skalkotos
wrote:
> Sorry, I don't get it. In inspect.c line 67 you have this:
> 
>   for (fs = fses; *fs; fs += 2) {
>     if (guestfs_int_check_for_filesystem_on (g, *fs)) {
>       guestfs_int_free_inspect_info (g);
>       return NULL;
>     }
>   }
Oh I see, yes that's a bug too.
> I don't see the bug.
I was looking at this other bug in src/inspect-fs.c:

  static int
  extend_fses (guestfs_h *g)
  {
    size_t n = g->nr_fses + 1;
    struct inspect_fs *p;

    p = realloc (g->fses, n * sizeof (struct inspect_fs));
    if (p == NULL) {
      perrorf (g, "realloc");
      return -1;
    }

Should probably just call safe_realloc ..

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

In inspect-fs-unix.c:1197 in add_fstab_entry() then too.


On 02/06/15 18:39, Richard W.M. Jones wrote:
> On Tue, Jun 02, 2015 at 06:37:06PM +0300, Nikos Skalkotos wrote:
>> Sorry, I don't get it. In inspect.c line 67 you have this:
>>
>>   for (fs = fses; *fs; fs += 2) {
>>     if (guestfs_int_check_for_filesystem_on (g, *fs)) {
>>       guestfs_int_free_inspect_info (g);
>>       return NULL;
>>     }
>>   }
> Oh I see, yes that's a bug too.
>
>> I don't see the bug.
> I was looking at this other bug in src/inspect-fs.c:
>
>   static int
>   extend_fses (guestfs_h *g)
>   {
>     size_t n = g->nr_fses + 1;
>     struct inspect_fs *p;
>
>     p = realloc (g->fses, n * sizeof (struct inspect_fs));
>     if (p == NULL) {
>       perrorf (g, "realloc");
>       return -1;
>     }
>
> Should probably just call safe_realloc ..
>
> Rich.
>