Merge pull request #2511 from 2xsec/coverity

src/lxc/pam/pam_cgfs.c

@@ -275,7 +275,7 @@ static bool mkdir_parent(const char *root, char *path)
 			goto next;
 
 		if (do_mkdir(path, 0755) < 0) {
-			pam_cgfs_debug("Failed to create %s: %s.\n", path, strerror(errno));
+			pam_cgfs_debug("Failed to create %s: %s\n", path, strerror(errno));
 			return false;
 		}
 
@@ -356,9 +356,8 @@ static int append_null_to_list(void ***list)
 	int newentry = 0;
 
 	if (*list)
-		for (; (*list)[newentry]; newentry++) {
+		for (; (*list)[newentry]; newentry++)
 			;
-		}
 
 	*list = must_realloc(*list, (newentry + 2) * sizeof(void **));
 	(*list)[newentry + 1] = NULL;
@@ -541,7 +540,7 @@ static int recursive_rmdir(char *dirname)
 
 		if (lstat(pathname, &st)) {
 			if (!r)
-				pam_cgfs_debug("Failed to stat %s.\n", pathname);
+				pam_cgfs_debug("Failed to stat %s\n", pathname);
 			r = -1;
 			goto next;
 		}
@@ -551,19 +550,20 @@ static int recursive_rmdir(char *dirname)
 
 		if (recursive_rmdir(pathname) < 0)
 			r = -1;
+
 next:
 		free(pathname);
 	}
 
 	if (rmdir(dirname) < 0) {
 		if (!r)
-			pam_cgfs_debug("Failed to delete %s: %s.\n", dirname, strerror(errno));
+			pam_cgfs_debug("Failed to delete %s: %s\n", dirname, strerror(errno));
 		r = -1;
 	}
 
 	if (closedir(dir) < 0) {
 		if (!r)
-			pam_cgfs_debug("Failed to delete %s: %s.\n", dirname, strerror(errno));
+			pam_cgfs_debug("Failed to delete %s: %s\n", dirname, strerror(errno));
 		r = -1;
 	}
 
@@ -730,6 +730,7 @@ static bool cgv1_controller_list_is_dup(struct cgv1_hierarchy **hlist, char **cl
 		if ((*it)->controllers)
 			if (cgv1_controller_lists_intersect((*it)->controllers, clist))
 				return true;
+
 	return false;
 
 }
@@ -744,7 +745,7 @@ static void cgv1_mark_to_make_rw(char **clist)
 	for (it = cgv1_hierarchies; it && *it; it++)
 		if ((*it)->controllers)
 			if (cgv1_controller_lists_intersect((*it)->controllers, clist) ||
-				string_in_list(clist, "all"))
+			    string_in_list(clist, "all"))
 				(*it)->create_rw_cgroup = true;
 }
 
@@ -776,8 +777,10 @@ static char *cgv1_must_prefix_named(char *entry)
 	s = must_alloc(len + 6);
 
 	ret = snprintf(s, len + 6, "name=%s", entry);
-	if (ret < 0 || (size_t)ret >= (len + 6))
+	if (ret < 0 || (size_t)ret >= (len + 6)) {
+		free(s);
 		return NULL;
+	}
 
 	return s;
 }
@@ -823,8 +826,6 @@ static char **cgv1_get_proc_mountinfo_controllers(char **klist, char **nlist, ch
 			return NULL;
 		p++;
 	}
-	if (!p)
-		return NULL;
 
 	if (strncmp(p, "/sys/fs/cgroup/", 15) != 0)
 		return NULL;
@@ -864,6 +865,7 @@ static bool cgv1_controller_in_clist(char *cgline, char *c)
 		if (strcmp(tok, c) == 0)
 			return true;
 	}
+
 	return false;
 }
 
@@ -935,6 +937,7 @@ static void cgv1_add_controller(char **clist, char *mountpoint, char *base_cgrou
 	int newentry;
 
 	new = must_alloc(sizeof(*new));
+
 	new->controllers = clist;
 	new->mountpoint = mountpoint;
 	new->base_cgroup = base_cgroup;
@@ -961,6 +964,7 @@ static void cgv2_add_controller(char **clist, char *mountpoint, char *base_cgrou
 	int newentry;
 
 	new = must_alloc(sizeof(*new));
+
 	new->controllers = clist;
 	new->mountpoint = mountpoint;
 	new->base_cgroup = base_cgroup;
@@ -1002,6 +1006,7 @@ static bool cg_systemd_under_user_slice_1(const char *in, uid_t uid)
 		p--;
 	if (p < copy)
 		goto cleanup;
+
 	/* make sure it is something.session */
 	len = strlen(p + 1);
 	if (len < strlen("1.session") ||
@@ -1012,6 +1017,7 @@ static bool cg_systemd_under_user_slice_1(const char *in, uid_t uid)
 	*(p + 1) = '\0';
 	while (p >= copy && *(--p) != '/')
 		;
+
 	if (sscanf(p + 1, "%d.user/", &id) != 1)
 		goto cleanup;
 
@@ -1121,6 +1127,7 @@ static bool cg_systemd_created_user_slice(const char *base_cgroup,
 
 succeed:
 	bret = true;
+
 cleanup:
 	free(copy);
 	return bret;
@@ -1142,9 +1149,9 @@ static bool cg_systemd_chown_existing_cgroup(const char *mountpoint,
 	 * need to chown it.
 	 */
 	if (chown(path, uid, gid) < 0)
-		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s.\n",
-			 path, (int)uid, (int)gid, strerror(errno), NULL);
-	pam_cgfs_debug("Chowned %s to %d:%d.\n", path, (int)uid, (int)gid);
+		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s\n",
+		         path, (int)uid, (int)gid, strerror(errno), NULL);
+	pam_cgfs_debug("Chowned %s to %d:%d\n", path, (int)uid, (int)gid);
 
 	free(path);
 	return true;
@@ -1183,8 +1190,7 @@ static bool cgv1_init(uid_t uid, gid_t gid)
 		if (!controller_list)
 			continue;
 
-		if (cgv1_controller_list_is_dup(cgv1_hierarchies,
-						controller_list)) {
+		if (cgv1_controller_list_is_dup(cgv1_hierarchies, controller_list)) {
 			free(controller_list);
 			continue;
 		}
@@ -1201,13 +1207,14 @@ static bool cgv1_init(uid_t uid, gid_t gid)
 			free(mountpoint);
 			continue;
 		}
+
 		trim(base_cgroup);
 		pam_cgfs_debug("Detected cgroupfs v1 controller \"%s\" with "
-			    "mountpoint \"%s\" and cgroup \"%s\".\n",
-			    controller_list[0], mountpoint, base_cgroup);
-		cgv1_add_controller(controller_list, mountpoint, base_cgroup,
-				    NULL);
+		               "mountpoint \"%s\" and cgroup \"%s\"\n",
+		               controller_list[0], mountpoint, base_cgroup);
+		cgv1_add_controller(controller_list, mountpoint, base_cgroup, NULL);
 	}
+
 	free_string_list(klist);
 	free_string_list(nlist);
 	free(basecginfo);
@@ -1222,6 +1229,7 @@ static bool cgv1_init(uid_t uid, gid_t gid)
 	for (it = cgv1_hierarchies; it && *it; it++) {
 		if ((*it)->controllers) {
 			char *init_cgroup, *user_slice;
+
 			/* We've already stored the controller and received its
 			 * current cgroup. If we now fail to retrieve its init
 			 * cgroup, we should probably fail.
@@ -1231,17 +1239,21 @@ static bool cgv1_init(uid_t uid, gid_t gid)
 				free(basecginfo);
 				return false;
 			}
+
 			cg_systemd_prune_init_scope(init_cgroup);
 			(*it)->init_cgroup = init_cgroup;
 			pam_cgfs_debug("cgroupfs v1 controller \"%s\" has init "
-				    "cgroup \"%s\".\n",
-				    (*(*it)->controllers), init_cgroup);
+			               "cgroup \"%s\"\n",
+			               (*(*it)->controllers), init_cgroup);
+
 			/* Check whether systemd has already created a cgroup
 			 * for us.
 			 */
 			user_slice = must_make_path((*it)->mountpoint, (*it)->base_cgroup, NULL);
 			if (cg_systemd_created_user_slice((*it)->base_cgroup, (*it)->init_cgroup, user_slice, uid))
 				(*it)->systemd_user_slice = true;
+
+			free(user_slice);
 		}
 	}
 	free(basecginfo);
@@ -1289,6 +1301,7 @@ static bool cgv2_init(uid_t uid, gid_t gid)
 		 */
 		goto cleanup;
 	}
+
 	cg_systemd_prune_init_scope(init_cgroup);
 
 	/* Check if the v2 hierarchy is mounted at its standard location.
@@ -1323,6 +1336,7 @@ static bool cgv2_init(uid_t uid, gid_t gid)
 	while (getline(&line, &len, f) != -1) {
 		char *user_slice;
 		bool has_user_slice = false;
+
 		if (!is_cgv2(line))
 			continue;
 
@@ -1336,6 +1350,7 @@ static bool cgv2_init(uid_t uid, gid_t gid)
 		free(user_slice);
 
 		cgv2_add_controller(NULL, mountpoint, current_cgroup, init_cgroup, has_user_slice);
+
 		/* Although the unified hierarchy can be mounted multiple times,
 		 * each of those mountpoints will expose identical information.
 		 * So let the first mountpoint we find, win.
@@ -1345,14 +1360,19 @@ static bool cgv2_init(uid_t uid, gid_t gid)
 	}
 
 	pam_cgfs_debug("Detected cgroupfs v2 hierarchy at mountpoint \"%s\" with "
-		    "current cgroup \"%s\" and init cgroup \"%s\".\n",
-		    mountpoint, current_cgroup, init_cgroup);
+	               "current cgroup \"%s\" and init cgroup \"%s\"\n",
+	               mountpoint, current_cgroup, init_cgroup);
 
 cleanup:
 	if (f)
 		fclose(f);
 	free(line);
 
+	if (!ret) {
+		free(init_cgroup);
+		free(current_cgroup);
+	}
+
 	return ret;
 }
 
@@ -1373,16 +1393,16 @@ static bool cg_init(uid_t uid, gid_t gid)
 
 	if (cgv1_hierarchies && cgv2_hierarchies) {
 		cg_mount_mode = CGROUP_MIXED;
-		pam_cgfs_debug("%s\n", "Detected cgroupfs v1 and v2 hierarchies.");
+		pam_cgfs_debug("%s\n", "Detected cgroupfs v1 and v2 hierarchies");
 	} else if (cgv1_hierarchies && !cgv2_hierarchies) {
 		cg_mount_mode = CGROUP_PURE_V1;
-		pam_cgfs_debug("%s\n", "Detected cgroupfs v1 hierarchies.");
+		pam_cgfs_debug("%s\n", "Detected cgroupfs v1 hierarchies");
 	} else if (cgv2_hierarchies && !cgv1_hierarchies) {
 		cg_mount_mode = CGROUP_PURE_V2;
-		pam_cgfs_debug("%s\n", "Detected cgroupfs v2 hierarchies.");
+		pam_cgfs_debug("%s\n", "Detected cgroupfs v2 hierarchies");
 	} else {
 		cg_mount_mode = CGROUP_UNKNOWN;
-		mysyslog(LOG_ERR, "Could not detect cgroupfs hierarchy.\n", NULL);
+		mysyslog(LOG_ERR, "Could not detect cgroupfs hierarchy\n", NULL);
 	}
 
 	if (cg_mount_mode == CGROUP_UNKNOWN)
@@ -1429,15 +1449,18 @@ static bool cgv1_enter(const char *cgroup)
 						      "/tasks",
 						      NULL);
 			}
-			pam_cgfs_debug("Attempting to enter cgroupfs v1 hierarchy in \"%s\" cgroup.\n", path);
+
+			pam_cgfs_debug("Attempting to enter cgroupfs v1 hierarchy in \"%s\" cgroup\n", path);
 			entered = write_int(path, (int)getpid());
 			if (entered) {
 				free(path);
 				break;
 			}
-			pam_cgfs_debug("Failed to enter cgroupfs v1 hierarchy in \"%s\" cgroup.\n", path);
+
+			pam_cgfs_debug("Failed to enter cgroupfs v1 hierarchy in \"%s\" cgroup\n", path);
 			free(path);
 		}
+
 		if (!entered)
 			return false;
 	}
@@ -1464,10 +1487,11 @@ static bool cgv2_enter(const char *cgroup)
 		return true;
 
 	path = must_make_path(v2->mountpoint, v2->base_cgroup, cgroup, "/cgroup.procs", NULL);
-	pam_cgfs_debug("Attempting to enter cgroupfs v2 hierarchy in cgroup \"%s\".\n", path);
+	pam_cgfs_debug("Attempting to enter cgroupfs v2 hierarchy in cgroup \"%s\"\n", path);
+
 	entered = write_int(path, (int)getpid());
 	if (!entered) {
-		pam_cgfs_debug("Failed to enter cgroupfs v2 hierarchy in cgroup \"%s\".\n", path);
+		pam_cgfs_debug("Failed to enter cgroupfs v2 hierarchy in cgroup \"%s\"\n", path);
 		free(path);
 		return false;
 	}
@@ -1481,12 +1505,12 @@ static bool cgv2_enter(const char *cgroup)
 static bool cg_enter(const char *cgroup)
 {
 	if (!cgv1_enter(cgroup)) {
-		mysyslog(LOG_WARNING, "cgroupfs v1: Failed to enter cgroups.\n", NULL);
+		mysyslog(LOG_WARNING, "cgroupfs v1: Failed to enter cgroups\n", NULL);
 		return false;
 	}
 
 	if (!cgv2_enter(cgroup)) {
-		mysyslog(LOG_WARNING, "cgroupfs v2: Failed to enter cgroups.\n", NULL);
+		mysyslog(LOG_WARNING, "cgroupfs v2: Failed to enter cgroups\n", NULL);
 		return false;
 	}
 
@@ -1505,17 +1529,17 @@ static void cgv1_escape(void)
 	 */
 	for (it = cgv1_hierarchies; it && *it; it++)
 		if (!cgv1_handle_root_cpuset_hierarchy(*it))
-			mysyslog(LOG_WARNING, "cgroupfs v1: Failed to initialize cpuset.\n", NULL);
+			mysyslog(LOG_WARNING, "cgroupfs v1: Failed to initialize cpuset\n", NULL);
 
 	if (!cgv1_enter("/"))
-		mysyslog(LOG_WARNING, "cgroupfs v1: Failed to escape to init's cgroup.\n", NULL);
+		mysyslog(LOG_WARNING, "cgroupfs v1: Failed to escape to init's cgroup\n", NULL);
 }
 
 /* Escape to root cgroup in the cgroupfs v2 hierarchy. */
 static void cgv2_escape(void)
 {
 	if (!cgv2_enter("/"))
-		mysyslog(LOG_WARNING, "cgroupfs v2: Failed to escape to init's cgroup.\n", NULL);
+		mysyslog(LOG_WARNING, "cgroupfs v2: Failed to escape to init's cgroup\n", NULL);
 }
 
 /* Wrapper around cgv{1,2}_escape(). */
@@ -1546,7 +1570,7 @@ static bool get_uid_gid(const char *user, uid_t *uid, gid_t *gid)
 	if (!pwentp) {
 		if (ret == 0)
 			mysyslog(LOG_ERR,
-				 "Could not find matched password record\n", NULL);
+			         "Could not find matched password record\n", NULL);
 
 		free(buf);
 		return false;
@@ -1598,6 +1622,7 @@ static uint32_t *cg_cpumask(char *buf, size_t nbits)
 		char *range = strchr(token, '-');
 		if (range)
 			end = strtoul(range + 1, NULL, 0);
+
 		if (!(start <= end)) {
 			free(bitarr);
 			return NULL;
@@ -1641,6 +1666,7 @@ static char *string_join(const char *sep, const char **parts, bool use_as_prefix
 	for (p = (char **)parts; *p; p++) {
 		if (p > (char **)parts)
 			(void)strlcat(result, sep, buf_len * sizeof(char));
+
 		(void)strlcat(result, *p, buf_len * sizeof(char));
 	}
 
@@ -1666,9 +1692,11 @@ static char *cg_cpumask_to_cpulist(uint32_t *bitarr, size_t nbits)
 				free_string_list(cpulist);
 				return NULL;
 			}
+
 			must_append_string(&cpulist, numstr);
 		}
 	}
+
 	return string_join(",", (const char **)cpulist, false);
 }
 
@@ -1691,10 +1719,12 @@ static ssize_t cg_get_max_cpus(char *cpulist)
 	else if (c1 < c2)
 		c1 = c2;
 
+	if (!c1)
+		return -1;
+
 	/* If the above logic is correct, c1 should always hold a valid string
 	 * here.
 	 */
-
 	errno = 0;
 	cpus = strtoul(c1, NULL, 0);
 	if (errno != 0)
@@ -1706,10 +1736,12 @@ static ssize_t cg_get_max_cpus(char *cpulist)
 static ssize_t write_nointr(int fd, const void* buf, size_t count)
 {
 	ssize_t ret;
+
 again:
 	ret = write(fd, buf, count);
 	if (ret < 0 && errno == EINTR)
 		goto again;
+
 	return ret;
 }
 
@@ -1721,16 +1753,19 @@ static int write_to_file(const char *filename, const void* buf, size_t count, bo
 	fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, 0666);
 	if (fd < 0)
 		return -1;
+
 	ret = write_nointr(fd, buf, count);
 	if (ret < 0)
 		goto out_error;
 	if ((size_t)ret != count)
 		goto out_error;
+
 	if (add_newline) {
 		ret = write_nointr(fd, "\n", 1);
 		if (ret != 1)
 			goto out_error;
 	}
+
 	close(fd);
 	return 0;
 
@@ -1755,15 +1790,17 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
 
 	lastslash = strrchr(path, '/');
 	if (!lastslash) { // bug...  this shouldn't be possible
-		pam_cgfs_debug("Invalid path: %s.\n", path);
+		pam_cgfs_debug("Invalid path: %s\n", path);
 		return bret;
 	}
+
 	oldv = *lastslash;
 	*lastslash = '\0';
+
 	fpath = must_make_path(path, "cpuset.cpus", NULL);
 	posscpus = read_file(fpath);
 	if (!posscpus) {
-		pam_cgfs_debug("Could not read file: %s.\n", fpath);
+		pam_cgfs_debug("Could not read file: %s\n", fpath);
 		goto on_error;
 	}
 
@@ -1774,16 +1811,18 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
 
 	if (!file_exists(__ISOL_CPUS)) {
 		/* This system doesn't expose isolated cpus. */
-		pam_cgfs_debug("%s", "Path: "__ISOL_CPUS" to read isolated cpus from does not exist.\n");
+		pam_cgfs_debug("%s", "Path: "__ISOL_CPUS" to read isolated cpus from does not exist\n");
 		cpulist = posscpus;
+
 		/* No isolated cpus but we weren't already initialized by
 		 * someone. We should simply copy the parents cpuset.cpus
 		 * values.
 		 */
 		if (!am_initialized) {
-			pam_cgfs_debug("%s", "Copying cpuset of parent cgroup.\n");
+			pam_cgfs_debug("%s", "Copying cpuset of parent cgroup\n");
 			goto copy_parent;
 		}
+
 		/* No isolated cpus but we were already initialized by someone.
 		 * Nothing more to do for us.
 		 */
@@ -1795,17 +1834,20 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
 		pam_cgfs_debug("%s", "Could not read file "__ISOL_CPUS"\n");
 		goto on_error;
 	}
+
 	if (!isdigit(isolcpus[0])) {
-		pam_cgfs_debug("%s", "No isolated cpus detected.\n");
+		pam_cgfs_debug("%s", "No isolated cpus detected\n");
 		cpulist = posscpus;
+
 		/* No isolated cpus but we weren't already initialized by
 		 * someone. We should simply copy the parents cpuset.cpus
 		 * values.
 		 */
 		if (!am_initialized) {
-			pam_cgfs_debug("%s", "Copying cpuset of parent cgroup.\n");
+			pam_cgfs_debug("%s", "Copying cpuset of parent cgroup\n");
 			goto copy_parent;
 		}
+
 		/* No isolated cpus but we were already initialized by someone.
 		 * Nothing more to do for us.
 		 */
@@ -1823,13 +1865,13 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
 
 	possmask = cg_cpumask(posscpus, maxposs);
 	if (!possmask) {
-		pam_cgfs_debug("%s", "Could not create cpumask for all possible cpus.\n");
+		pam_cgfs_debug("%s", "Could not create cpumask for all possible cpus\n");
 		goto on_error;
 	}
 
 	isolmask = cg_cpumask(isolcpus, maxposs);
 	if (!isolmask) {
-		pam_cgfs_debug("%s", "Could not create cpumask for all isolated cpus.\n");
+		pam_cgfs_debug("%s", "Could not create cpumask for all isolated cpus\n");
 		goto on_error;
 	}
 
@@ -1841,23 +1883,27 @@ static bool cg_filter_and_set_cpus(char *path, bool am_initialized)
 	}
 
 	if (!flipped_bit) {
-		pam_cgfs_debug("%s", "No isolated cpus present in cpuset.\n");
+		pam_cgfs_debug("%s", "No isolated cpus present in cpuset\n");
 		goto on_success;
 	}
-	pam_cgfs_debug("%s", "Removed isolated cpus from cpuset.\n");
+	pam_cgfs_debug("%s", "Removed isolated cpus from cpuset\n");
 
 	cpulist = cg_cpumask_to_cpulist(possmask, maxposs);
 	if (!cpulist) {
-		pam_cgfs_debug("%s", "Could not create cpu list.\n");
+		pam_cgfs_debug("%s", "Could not create cpu list\n");
 		goto on_error;
 	}
 
 copy_parent:
 	*lastslash = oldv;
+
+	if (fpath)
+		free(fpath);
+
 	fpath = must_make_path(path, "cpuset.cpus", NULL);
 	ret = write_to_file(fpath, cpulist, strlen(cpulist), false);
 	if (ret < 0) {
-		pam_cgfs_debug("Could not write cpu list to: %s.\n", fpath);
+		pam_cgfs_debug("Could not write cpu list to: %s\n", fpath);
 		goto on_error;
 	}
 
@@ -1865,8 +1911,9 @@ on_success:
 	bret = true;
 
 on_error:
-	free(fpath);
+	*lastslash = oldv;
 
+	free(fpath);
 	free(isolcpus);
 	free(isolmask);
 
@@ -1890,6 +1937,7 @@ int read_from_file(const char *filename, void* buf, size_t count)
 	if (!buf || !count) {
 		char buf2[100];
 		size_t count2 = 0;
+
 		while ((ret = read(fd, buf2, 100)) > 0)
 			count2 += ret;
 		if (ret >= 0)
@@ -1920,21 +1968,27 @@ static bool cg_copy_parent_file(char *path, char *file)
 		pam_cgfs_debug("cgfsng:copy_parent_file: bad path %s", path);
 		return false;
 	}
+
 	oldv = *lastslash;
 	*lastslash = '\0';
+
 	fpath = must_make_path(path, file, NULL);
 	len = read_from_file(fpath, NULL, 0);
 	if (len <= 0)
 		goto bad;
+
 	value = must_alloc(len + 1);
 	if (read_from_file(fpath, value, len) != len)
 		goto bad;
 	free(fpath);
+
 	*lastslash = oldv;
+
 	fpath = must_make_path(path, file, NULL);
 	ret = write_to_file(fpath, value, len, false);
 	if (ret < 0)
 		pam_cgfs_debug("Unable to write %s to %s", value, fpath);
+
 	free(fpath);
 	free(value);
 	return ret >= 0;
@@ -1977,6 +2031,7 @@ static bool cgv1_handle_root_cpuset_hierarchy(struct cgv1_hierarchy *h)
 		free(clonechildrenpath);
 		return false;
 	}
+
 	free(clonechildrenpath);
 	return true;
 }
@@ -2004,17 +2059,20 @@ static bool cgv1_handle_cpuset_hierarchy(struct cgv1_hierarchy *h,
 	cgpath = must_make_path(h->mountpoint, h->base_cgroup, cgroup, NULL);
 	if (slash)
 		*slash = '/';
+
 	if (do_mkdir(cgpath, 0755) < 0 && errno != EEXIST) {
 		pam_cgfs_debug("Failed to create '%s'", cgpath);
 		free(cgpath);
 		return false;
 	}
+
 	clonechildrenpath = must_make_path(cgpath, "cgroup.clone_children", NULL);
 	if (!file_exists(clonechildrenpath)) { /* unified hierarchy doesn't have clone_children */
 		free(clonechildrenpath);
 		free(cgpath);
 		return true;
 	}
+
 	if (read_from_file(clonechildrenpath, &v, 1) < 0) {
 		pam_cgfs_debug("Failed to read '%s'", clonechildrenpath);
 		free(clonechildrenpath);
@@ -2024,14 +2082,14 @@ static bool cgv1_handle_cpuset_hierarchy(struct cgv1_hierarchy *h,
 
 	/* Make sure any isolated cpus are removed from cpuset.cpus. */
 	if (!cg_filter_and_set_cpus(cgpath, v == '1')) {
-		pam_cgfs_debug("%s", "Failed to remove isolated cpus.\n");
+		pam_cgfs_debug("%s", "Failed to remove isolated cpus\n");
 		free(clonechildrenpath);
 		free(cgpath);
 		return false;
 	}
 
 	if (v == '1') {  /* already set for us by someone else */
-		pam_cgfs_debug("%s", "\"cgroup.clone_children\" was already set to \"1\".\n");
+		pam_cgfs_debug("%s", "\"cgroup.clone_children\" was already set to \"1\"\n");
 		free(clonechildrenpath);
 		free(cgpath);
 		return true;
@@ -2039,7 +2097,7 @@ static bool cgv1_handle_cpuset_hierarchy(struct cgv1_hierarchy *h,
 
 	/* copy parent's settings */
 	if (!cg_copy_parent_file(cgpath, "cpuset.mems")) {
-		pam_cgfs_debug("%s", "Failed to copy \"cpuset.mems\" settings.\n");
+		pam_cgfs_debug("%s", "Failed to copy \"cpuset.mems\" settings\n");
 		free(cgpath);
 		free(clonechildrenpath);
 		return false;
@@ -2072,6 +2130,7 @@ static bool cgv1_create_one(struct cgv1_hierarchy *h, const char *cgroup, uid_t
 
 	*existed = false;
 	it = h;
+
 	for (controller = it->controllers; controller && *controller;
 	     controller++) {
 		if (!cgv1_handle_cpuset_hierarchy(it, cgroup))
@@ -2082,9 +2141,8 @@ static bool cgv1_create_one(struct cgv1_hierarchy *h, const char *cgroup, uid_t
 		 */
 		if (cg_systemd_chown_existing_cgroup(it->mountpoint,
 						     it->base_cgroup, uid, gid,
-						     it->systemd_user_slice)) {
+						     it->systemd_user_slice))
 			return true;
-		}
 
 		/* We need to make sure that we do not create an endless chain
 		 * of sub-cgroups. So we check if we have already logged in
@@ -2106,27 +2164,34 @@ static bool cgv1_create_one(struct cgv1_hierarchy *h, const char *cgroup, uid_t
 		}
 
 		path = must_make_path(it->mountpoint, it->init_cgroup, cgroup, NULL);
-		pam_cgfs_debug("Constructing path: %s.\n", path);
+		pam_cgfs_debug("Constructing path: %s\n", path);
+
 		if (file_exists(path)) {
 			bool our_cg = cg_belongs_to_uid_gid(path, uid, gid);
-			pam_cgfs_debug("%s existed and does %shave our uid: %d and gid: %d.\n", path, our_cg ? "" : "not ", uid, gid);
-			free(path);
 			if (our_cg)
 				*existed = false;
 			else
 				*existed = true;
+
+			pam_cgfs_debug("%s existed and does %shave our uid: %d and gid: %d\n",
+			               path, our_cg ? "" : "not ", uid, gid);
+			free(path);
+
 			return our_cg;
 		}
+
 		created = mkdir_parent(it->mountpoint, path);
 		if (!created) {
 			free(path);
 			continue;
 		}
+
 		if (chown(path, uid, gid) < 0)
 			mysyslog(LOG_WARNING,
-				 "Failed to chown %s to %d:%d: %s.\n", path,
-				 (int)uid, (int)gid, strerror(errno), NULL);
-		pam_cgfs_debug("Chowned %s to %d:%d.\n", path, (int)uid, (int)gid);
+			         "Failed to chown %s to %d:%d: %s\n", path,
+			         (int)uid, (int)gid, strerror(errno), NULL);
+
+		pam_cgfs_debug("Chowned %s to %d:%d\n", path, (int)uid, (int)gid);
 		free(path);
 		break;
 	}
@@ -2255,12 +2320,12 @@ static bool cgv2_create(const char *cgroup, uid_t uid, gid_t gid, bool *existed)
 	}
 
 	path = must_make_path(v2->mountpoint, v2->base_cgroup, cgroup, NULL);
-	pam_cgfs_debug("Constructing path \"%s\".\n", path);
+	pam_cgfs_debug("Constructing path \"%s\"\n", path);
+
 	if (file_exists(path)) {
 		our_cg = cg_belongs_to_uid_gid(path, uid, gid);
-		pam_cgfs_debug(
-		    "%s existed and does %shave our uid: %d and gid: %d.\n",
-		    path, our_cg ? "" : "not ", uid, gid);
+		pam_cgfs_debug("%s existed and does %shave our uid: %d and gid: %d\n",
+		               path, our_cg ? "" : "not ", uid, gid);
 		free(path);
 		if (our_cg) {
 			*existed = false;
@@ -2279,10 +2344,10 @@ static bool cgv2_create(const char *cgroup, uid_t uid, gid_t gid, bool *existed)
 
 	/* chown cgroup to user */
 	if (chown(path, uid, gid) < 0)
-		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s.\n",
-			 path, (int)uid, (int)gid, strerror(errno), NULL);
+		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s\n",
+		         path, (int)uid, (int)gid, strerror(errno), NULL);
 	else
-		pam_cgfs_debug("Chowned %s to %d:%d.\n", path, (int)uid, (int)gid);
+		pam_cgfs_debug("Chowned %s to %d:%d\n", path, (int)uid, (int)gid);
 	free(path);
 
 delegate_files:
@@ -2293,12 +2358,13 @@ delegate_files:
 	else
 		path = must_make_path(v2->mountpoint, v2->base_cgroup, cgroup,
 				      "/cgroup.procs", NULL);
+
 	ret = chown(path, uid, gid);
 	if (ret < 0)
-		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s.\n",
-			 path, (int)uid, (int)gid, strerror(errno), NULL);
+		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s\n",
+		         path, (int)uid, (int)gid, strerror(errno), NULL);
 	else
-		pam_cgfs_debug("Chowned %s to %d:%d.\n", path, (int)uid, (int)gid);
+		pam_cgfs_debug("Chowned %s to %d:%d\n", path, (int)uid, (int)gid);
 	free(path);
 
 	/* chown cgroup.subtree_control to user */
@@ -2308,9 +2374,10 @@ delegate_files:
 	else
 		path = must_make_path(v2->mountpoint, v2->base_cgroup, cgroup,
 				      "/cgroup.subtree_control", NULL);
+
 	ret = chown(path, uid, gid);
 	if (ret < 0)
-		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s.\n",
+		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s\n",
 			 path, (int)uid, (int)gid, strerror(errno), NULL);
 	free(path);
 
@@ -2323,7 +2390,7 @@ delegate_files:
 				      "/cgroup.threads", NULL);
 	ret = chown(path, uid, gid);
 	if (ret < 0 && errno != ENOENT)
-		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s.\n",
+		mysyslog(LOG_WARNING, "Failed to chown %s to %d:%d: %s\n",
 			 path, (int)uid, (int)gid, strerror(errno), NULL);
 	free(path);
 
@@ -2344,7 +2411,7 @@ static int handle_login(const char *user, uid_t uid, gid_t gid)
 	while (idx >= 0) {
 		ret = snprintf(cg, MAXPATHLEN, "/user/%s/%d", user, idx);
 		if (ret < 0 || ret >= MAXPATHLEN) {
-			mysyslog(LOG_ERR, "Username too long.\n", NULL);
+			mysyslog(LOG_ERR, "Username too long\n", NULL);
 			return PAM_SESSION_ERR;
 		}
 
@@ -2355,7 +2422,8 @@ static int handle_login(const char *user, uid_t uid, gid_t gid)
 				idx++;
 				continue;
 			}
-			mysyslog(LOG_ERR, "Failed to create a cgroup for user %s.\n", user, NULL);
+
+			mysyslog(LOG_ERR, "Failed to create a cgroup for user %s\n", user, NULL);
 			return PAM_SESSION_ERR;
 		}
 
@@ -2366,14 +2434,16 @@ static int handle_login(const char *user, uid_t uid, gid_t gid)
 				idx++;
 				continue;
 			}
-			mysyslog(LOG_ERR, "Failed to create a cgroup for user %s.\n", user, NULL);
+
+			mysyslog(LOG_ERR, "Failed to create a cgroup for user %s\n", user, NULL);
 			return PAM_SESSION_ERR;
 		}
 
 		if (!cg_enter(cg)) {
-			mysyslog( LOG_ERR, "Failed to enter user cgroup %s for user %s.\n", cg, user, NULL);
+			mysyslog( LOG_ERR, "Failed to enter user cgroup %s for user %s\n", cg, user, NULL);
 			return PAM_SESSION_ERR;
 		}
+
 		break;
 	}
 
@@ -2403,7 +2473,8 @@ static bool cgv1_prune_empty_cgroups(const char *user)
 			bool path_base_rm, path_init_rm;
 
 			path_base = must_make_path((*it)->mountpoint, (*it)->base_cgroup, "/user", user, NULL);
-			pam_cgfs_debug("cgroupfs v1: Trying to prune \"%s\".\n", path_base);
+			pam_cgfs_debug("cgroupfs v1: Trying to prune \"%s\"\n", path_base);
+
 			ret = recursive_rmdir(path_base);
 			if (ret == -ENOENT || ret >= 0)
 				path_base_rm = true;
@@ -2412,7 +2483,8 @@ static bool cgv1_prune_empty_cgroups(const char *user)
 			free(path_base);
 
 			path_init = must_make_path((*it)->mountpoint, (*it)->init_cgroup, "/user", user, NULL);
-			pam_cgfs_debug("cgroupfs v1: Trying to prune \"%s\".\n", path_init);
+			pam_cgfs_debug("cgroupfs v1: Trying to prune \"%s\"\n", path_init);
+
 			ret = recursive_rmdir(path_init);
 			if (ret == -ENOENT || ret >= 0)
 				path_init_rm = true;
@@ -2428,6 +2500,7 @@ static bool cgv1_prune_empty_cgroups(const char *user)
 			controller_removed = true;
 			break;
 		}
+
 		if (!controller_removed)
 			all_removed = false;
 	}
@@ -2451,7 +2524,8 @@ static bool cgv2_prune_empty_cgroups(const char *user)
 	v2 = *cgv2_hierarchies;
 
 	path_base = must_make_path(v2->mountpoint, v2->base_cgroup, "/user", user, NULL);
-	pam_cgfs_debug("cgroupfs v2: Trying to prune \"%s\".\n", path_base);
+	pam_cgfs_debug("cgroupfs v2: Trying to prune \"%s\"\n", path_base);
+
 	ret = recursive_rmdir(path_base);
 	if (ret == -ENOENT || ret >= 0)
 		path_base_rm = true;
@@ -2460,7 +2534,8 @@ static bool cgv2_prune_empty_cgroups(const char *user)
 	free(path_base);
 
 	path_init = must_make_path(v2->mountpoint, v2->init_cgroup, "/user", user, NULL);
-	pam_cgfs_debug("cgroupfs v2: Trying to prune \"%s\".\n", path_init);
+	pam_cgfs_debug("cgroupfs v2: Trying to prune \"%s\"\n", path_init);
+
 	ret = recursive_rmdir(path_init);
 	if (ret == -ENOENT || ret >= 0)
 		path_init_rm = true;
@@ -2497,11 +2572,13 @@ static void cgv1_free_hierarchies(void)
 
 			free((*it)->controllers);
 		}
+
 		free((*it)->mountpoint);
 		free((*it)->base_cgroup);
 		free((*it)->fullcgpath);
 		free((*it)->init_cgroup);
 	}
+
 	free(cgv1_hierarchies);
 }
 
@@ -2516,16 +2593,19 @@ static void cgv2_free_hierarchies(void)
 	for (it = cgv2_hierarchies; it && *it; it++) {
 		if ((*it)->controllers) {
 			char **tmp;
+
 			for (tmp = (*it)->controllers; tmp && *tmp; tmp++)
 				free(*tmp);
 
 			free((*it)->controllers);
 		}
+
 		free((*it)->mountpoint);
 		free((*it)->base_cgroup);
 		free((*it)->fullcgpath);
 		free((*it)->init_cgroup);
 	}
+
 	free(cgv2_hierarchies);
 }
 
@@ -2551,7 +2631,7 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc,
 	}
 
 	if (!get_uid_gid(PAM_user, &uid, &gid)) {
-		mysyslog(LOG_ERR, "Failed to get uid and gid for %s.\n", PAM_user, NULL);
+		mysyslog(LOG_ERR, "Failed to get uid and gid for %s\n", PAM_user, NULL);
 		return PAM_SESSION_ERR;
 	}
 
@@ -2576,7 +2656,7 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc,
 		 * that simply doesn't make any sense.
 		 */
 		if (string_list_length(clist) > 1 && string_in_list(clist, "all")) {
-			mysyslog(LOG_ERR, "Invalid -c option, cannot specify individual controllers alongside 'all'.\n", NULL);
+			mysyslog(LOG_ERR, "Invalid -c option, cannot specify individual controllers alongside 'all'\n", NULL);
 			free_string_list(clist);
 			return PAM_SESSION_ERR;
 		}
@@ -2603,7 +2683,7 @@ int pam_sm_close_session(pam_handle_t *pamh, int flags, int argc,
 	}
 
 	if (!get_uid_gid(PAM_user, &uid, &gid)) {
-		mysyslog(LOG_ERR, "Failed to get uid and gid for %s.\n", PAM_user, NULL);
+		mysyslog(LOG_ERR, "Failed to get uid and gid for %s\n", PAM_user, NULL);
 		return PAM_SESSION_ERR;
 	}
 
@@ -2619,7 +2699,7 @@ int pam_sm_close_session(pam_handle_t *pamh, int flags, int argc,
 			 * that simply doesn't make any sense.
 			 */
 			if (string_list_length(clist) > 1 && string_in_list(clist, "all")) {
-				mysyslog(LOG_ERR, "Invalid -c option, cannot specify individual controllers alongside 'all'.\n", NULL);
+				mysyslog(LOG_ERR, "Invalid -c option, cannot specify individual controllers alongside 'all'\n", NULL);
 				free_string_list(clist);
 				return PAM_SESSION_ERR;
 			}