On (21/08/13 09:50), Pavel Březina wrote:
>On 08/19/2013 09:56 PM, Michal Židek wrote:
>>+static void  sss_mc_save_corrupted(struct sss_mc_ctx *mc_ctx)
>>+{
>>+    int err;
>>+    int fd = -1;
>>+    ssize_t written;
>>+    char *file = NULL;
>>+    TALLOC_CTX *tmp_ctx;
>>+
>>+    if (mc_ctx == NULL) {
>>+        DEBUG(SSSDBG_TRACE_FUNC,
>>+              ("Cannot store uninitialized cache. Nothing to do.\n"));
>>+        return;
>>+    }
>>+
>>+    tmp_ctx = talloc_new(NULL);
>>+    if (tmp_ctx == NULL) {
>>+        DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory.\n"));
>>+        return;
>>+    }
>>+
>>+    file = talloc_asprintf(tmp_ctx, "%s_%s",
>>+                           mc_ctx->file, "corrupted");
>>+    if (file == NULL) {
>>+        DEBUG(SSSDBG_CRIT_FAILURE, ("Out of memory.\n"));
>>+        goto done;
>>+    }
>>+
>>+    /* We will always store only the last problematic cache state */
>>+    fd = creat(file, 0600);
>>+    if (fd == -1) {
>>+        err = errno;
>>+        DEBUG(SSSDBG_CRIT_FAILURE,
>>+              ("Failed to open file '%s' [%d]: %s\n",
>>+               file, err, strerror(err)));
>>+        goto done;
>>+    }
>>+
>>+    written = write(fd, mc_ctx->mmap_base, mc_ctx->mmap_size);
>
>Shouldn't sss_atomic_write_s() be used?
>
>>+    if (written != mc_ctx->mmap_size) {
>>+        if (written == -1) {
>>+            err = errno;
>>+            DEBUG(SSSDBG_CRIT_FAILURE,
>>+                  ("write() failed [%d]: %s\n", err, strerror(err)));
>>+        } else {
>>+            DEBUG(SSSDBG_CRIT_FAILURE,
>>+                  ("write() returned %zd (expected (%zd))\n",
>>+                   written, mc_ctx->mmap_size));
>>+        }
>>+        goto done;
>>+    }
>
>I think the file should be unlinked in case of error.
>
>>+
>>+    sss_log(SSS_LOG_NOTICE,
>>+            "Stored copy of corrupted mmap cache in file '%s\n'", file);
>>+done:
>>+    if (fd != -1) {
>>+        close(fd);
>>+    }
>>+    talloc_free(tmp_ctx);
>>+}

I think that function sss_mc_save_corrupted will never be called with patches
from thread "[PATCHES] mmap_cache: Skip records which doesn't have same hash"

It is very likely that it is a dead code. I would prefer to ignore this
nitpicks, because patches have already been pushed.

LS
_______________________________________________
sssd-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to