search

clearlinux / swupd-server

Software update server (deprecated)
Other
13 stars 17 forks source link

Properly initialise includes for new manifests #24

Closed incandescant closed 8 years ago

incandescant commented 8 years ago

Initialise the includes member to null during alloc_manifest, otherwise when trying to access this member in a setup where manifest includes aren't used we segfault.

Signed-off-by: Joshua Lock joshua.g.lock@intel.com

incandescant commented 8 years ago

Apologies, I pulled the trigger on this too soon. This is a hunch I'm currently testing when trying to root cause the issue with segfaults mentioned in #22 (it sure smells like an uninitialised variable).

I'll comment here, or close the PR, once my testing is done.

ikeydoherty commented 8 years ago

Walk me through this please, that's a calloc call, the memory is already zero'd. There's no need that I can determine for the NULL assignment.

The real place the NULL assignment is missing is here: https://github.com/clearlinux/swupd-server/blob/master/src/manifest.c#L93

struct manifest *manifest;

should become:

struct manifest *manifest = NULL;
ikeydoherty commented 8 years ago

Do we not have a gdb trace for #22 @incandescant ?

incandescant commented 8 years ago

Thanks for the swift feedback @ikeydoherty, I should definitely have been paying more attention.

I hadn't meant to hit the "Create pull request" button before I'd finished testing...

Essentially this was a stab in the dark attempt to fix an issue we're seeing where swupd_create_update segfaults in our Yocto Project integration, we aren't using manifest includes (yet) and @igor-stoppa traced the segfault as discussed in #22.

I'm still looking at this so will close the PR for now.

ikeydoherty commented 8 years ago

@incandescant all good - I noticed the PR buttons/UX got a change overnight anyway.