784763 - [DeviceStorage]: freeBytes in stat() result is wrong

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[David Flanagan [:djf]]

When I stat() the photos directory and the look at freeBytes to find out if there is enough space, the number I get is wrong.

When the SD card on my otoro was full, it reported 42M free.

On desktop, with many GB free, it reported 53M free.

On both, the number it reported before a screenshot was saved was larger than the number it reported after the screenshot.  That is, saving a file increased the number of free bytes.

Interestingly, though the difference between the numbers was, in fact, the exact size of the file that had been saved (on the mac, at least.  Didn't check that on otoro).

I thought maybe the value being reported by freeBytes was actually the number of used bytes, but it doesn't look like those numbers add up.

When there are exactly 0 bytes free on /sdcard, freeBytes reports: 42659010 and totalBytes reports 18446744073709519000.  That's on a 16gb sdcard, so its off by orders of magnitude.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Few mistakes correct:

We had the parms reversed.
We are now am filtering out any dot files.
This simplifies and correctly calculating the disk space.

Comment 2

[Doug Turner (:dougt)]

s/calculating/calculates

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 656159 [details] [diff] [review]
patch v.1

Review of attachment 656159 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the second comment addressed

::: dom/devicestorage/DeviceStorageRequestParent.cpp
@@ +312,5 @@
>    NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
>  
>    nsCOMPtr<nsIRunnable> r;
> +  uint64_t diskUsage = 0;
> +  DeviceStorageFile::DirectoryDiskUsage(mFile->mFile, &diskUsage);

This is a little bit weird signature. A more common way to do it is to return the usage and then add things up as you recurse. I.e.

int DirectoryDiskUsage(directory, ...) {
  int total = 0;
  for (file in directory) {
    total += file.isFile ? file.size : DirectoryDiskUsage(file);
  }
  return total;
}

I'm ok either way, but would prefer something like the above.

::: dom/devicestorage/nsDeviceStorage.cpp
@@ +489,5 @@
> +      continue;
> +    }
> +
> +    // Ignore any file that begins with a dot
> +    if (StringBeginsWith(leafName, NS_LITERAL_STRING("."))) {

Why do you not include these? What if someone adds a picture called ".mypic.gif"?

Shouldn't you filter files by type? I.e. when getting usage file "pictures" we should use the same filter function as when doing the cursor enumeration. That should filter out .files as needed usually.

Comment 4

[Doug Turner (:dougt)]

DeviceStorageFile::DirectoryDiskUsage(mFile->mFile, &diskUsage);

I am using this style to match GetDiskSpaceAvailable() and to avoid having a default parameter.


> Why do you not include these (dotfiles)?

I'll remove the checks.

Thanks!

Comment 5

[Doug Turner (:dougt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0e2cd03631ce

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

You should still filter by type though, no? Otherwise we can report lots of using for the "pictures" DeviceStorage, while enumerating it returns no files due to the storage is actually being taken up by music files.

Comment 7

[Doug Turner (:dougt)]

see bug 787299.

Comment 8

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/0e2cd03631ce