Closed Bug 784763 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: DOM: Device Interfaces, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: djf, Assigned: dougt)

Details

Attachments

(1 file)

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.
Assignee: nobody → doug.turner
Attached patch patch v.1Splinter Review
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.
Attachment #656159 - Flags: review?(jonas)
s/calculating/calculates
Component: DOM → DOM: Device Interfaces
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.
Attachment #656159 - Flags: review?(jonas) → review+
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!
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.
see bug 787299.
https://hg.mozilla.org/mozilla-central/rev/0e2cd03631ce
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.