cfengine/core — pull request #316 by nyetsche (2013-01-29 20:11:07+00:00)
Title: check for non-null Promise *pp before dereferencing

This is a simple fix for a bug I've recently encountered involving a storage promise that edits fstab; cf-agent would segfault and die before returning /etc/fstab to existence, leaving /etc/fstab.cf-before-edit and /etc/fstab.cf-after-edit but no /etc/fstab.

The promise setup looks something like:

```storage:
"/home"
mount => nfs_p("nfs-server", "/nfs/path", "nfs,opts")

```

and `nfs_p` defined as:

```
body mount nfs_p(server,source,perm)
{
mount_type => "nfs";
mount_source => "$(source)";
mount_server => "$(server)";
mount_options => {"$(perm)"};
edit_fstab => "true";
}
```

The bug comes from this commit a few months ago:

commit 3ee952849d4b4c35ed363f00ab1997b4c0ac8e38
Author: Eystein Måløy Stenberg
Date: Mon Sep 10 16:08:38 2012 -0700

split CopyRegularFileDisk to make it more general-purpose - no need for dummy promise or attributes

Specifically the rewrite of CopyRegularFileDisk, called from ArchiveToRepository:

```
@@ -131,15 +131,9 @@ int ArchiveToRepository(char *file, Attributes attr, Promise *pp, const ReportCo

[...]

- if (CopyRegularFileDisk(file, destination, attr, pp))
+ if (CopyRegularFileDisk(file, destination, pp->makeholes))

-int CopyRegularFileDisk(char *source, char *new, Attributes attr, Promise *pp)
[...]
+bool CopyRegularFileDisk(char *source, char *destination, bool make_holes)

@@ -182,7 +202,7 @@ int CopyRegularFileDisk(char *source, char *new, Attributes attr, Promise *pp)

intp = 0;

- if (pp && pp->makeholes)
+ if (make_holes)
```
The pull request here (naively?) uses a similar simple check - we make sure that Promise *pp in ArchiveToRepository is not null. I'll confess to not really understanding what the Promise *pp structure does, so I'm not sure if the fact that it is NULL here means there is another underlying bug somewhere else.

With this commit cf-agent no longer segfaults on a nfs mount+fstab edit. There may be other places where cf-agent segfaults that this fixes, I'm not sure.

I also feel that it's a bug that /etc/fstab is unlinked without the promise succeeding, although I haven't figured out a fix for that.