[pacman-dev] Disk space checking branch "complete"

older
[pacman-dev] [PATCH 1/2] dirent...

Allan McRae

15 Nov 2010 15 Nov '10

5:33 p.m.

Hi, I have "finished" the disk space checking implementation. The only TODO left is to add the statvfs version of getmntinfo code path (NetBSD), but otherwise I consider this done. The calculations done are quite good but it is near impossible to be exact (e.g. adding/removing a block for directories but only once across the transaction, .pacsave/.pacnew files, filelist db files). I suppose I could add a block for those and not remove blocks to get an upper limit. However, the difference will likely be orders of magnitude smaller than the transaction size so I am not sure it is worth further effort... As far as the interface goes, the check is performed only when the CheckSpace option is given in pacman.conf. We also get a nice progress bar in pacman while the check is being performed (think of what is displayed during file conflict checking). See the work here: http://projects.archlinux.org/users/allan/pacman.git/diff/?h=diskspace&id2=working I need to rebase that branch into useful commit(s), but the overall changes are rather small (12 files changed, 447 insertions, 2 deletions - with 365 of those additions being in two new files). So I think code review can happen now. I am looking for comments on code correctness and very specific implementation details only. Anything other than that needs to be submitted in patch form... Allan

Show replies by date

Allan McRae

16 Nov 16 Nov

7:18 a.m.

On 16/11/10 03:33, Allan McRae wrote:

...

See the work here: http://projects.archlinux.org/users/allan/pacman.git/diff/?h=diskspace&id2=working

I need to rebase that branch into useful commit(s), but the overall changes are rather small (12 files changed, 447 insertions, 2 deletions - with 365 of those additions being in two new files). So I think code review can happen now.

OK, rebasing done: http://projects.archlinux.org/users/allan/pacman.git/?h=diskspace If anyone who is going to review them actually wants the patch series posted here, I will happily forward it... Allan

Bryce Gibson

8:02 a.m.

Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation. ----------------original message----------------- From: "Allan McRae" allan@archlinux.org To: "Discussion list for pacman development" Date: Tue, 16 Nov 2010 17:18:43 +1000 -------------------------------------------------

...

On 16/11/10 03:33, Allan McRae wrote:

...
See the work here:

http://projects.archlinux.org/users/allan/pacman.git/diff/?h=disks pace&id2=working

I need to rebase that branch into useful commit(s), but the overall changes are rather small (12 files changed, 447 insertions, 2 deletions - with 365 of those additions being in two new files). So I think code review can happen now.

OK, rebasing done: http://projects.archlinux.org/users/allan/pacman.git/?h=diskspace

If anyone who is going to review them actually wants the patch series posted here, I will happily forward it...

Allan

-- Please note this email and any attachments are intended exclusively for the use of the company or individual shown as the addressee(s). If you have reason to believe you are not the intended recipient please notify the sender by return email immediately and destroy the message you received without making any copies. Any copying, interference or disclosure of this message is therefore unauthorised and expressly prohibited. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Gibson Consulting Services P/L. We do not warrant that this message is virus free. Please perform your own virus check before opening any attachment.

Bryce Gibson

8:07 a.m.

...

Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

...

----------------original message----------------- From: "Allan McRae" allan@archlinux.org To: "Discussion list for pacman development" Date: Tue, 16 Nov 2010 17:18:43 +1000 -------------------------------------------------

...
On 16/11/10 03:33, Allan McRae wrote:

...
See the work here:

http://projects.archlinux.org/users/allan/pacman.git/diff/?h=dis ks pace&id2=working

I need to rebase that branch into useful commit(s), but the overall changes are rather small (12 files changed, 447 insertions, 2 deletions - with 365 of those additions being in two new files). So I think code review can happen now.

OK, rebasing done:

http://projects.archlinux.org/users/allan/pacman.git/?h=diskspace

If anyone who is going to review them actually wants the patch series posted here, I will happily forward it...

Allan

-- Please note this email and any attachments are intended exclusively for

...

use of the company or individual shown as the addressee(s). If you have reason to believe you are not the intended recipient please notify the sender by return email immediately and destroy the message you received without making any copies. Any copying, interference or disclosure of this message is therefore unauthorised and expressly prohibited. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent

Or perhaps a command line flag as another option... the those

...

of Gibson Consulting Services P/L. We do not warrant that this message is virus free. Please perform your own virus check before opening any attachment.

Allan McRae

11:03 a.m.

On 16/11/10 18:02, Bryce Gibson wrote:

...

Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems. Allan

Bryce Gibson

1:12 p.m.

...

On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

Just FYI :) One that should succeed: df -h /asd Filesystem Size Used Avail Use% Mounted on /dev/root 463M 381M 82M 83% / src/pacman/pacman -S --debug nano --dbpath `pwd`/db --root /asd ... Targets (5): linux-api-headers-2.6.34-1 tzdata-2010o-1 glibc-2.12.1-4 ncurses-5.7-4 nano-2.2.5-1 Total Download Size: 0.00 MB Total Installed Size: 50.53 MB Proceed with installation? [Y/n] y ... debug: checking available disk space ... debug: partition /, needed 12719, free 20766 debug: installing packages Or one that "should" fail (I think it _just_ doesn't fit in reality, seems to fail on the last package, but now I've crashed my machine somewhat, so I can't confirm for certain): df -h /asd Filesystem Size Used Avail Use% Mounted on /dev/root 463M 381M 82M 83% / src/pacman/pacman -S --debug python nano --dbpath `pwd`/db --root /asd ... Targets (24): linux-api-headers-2.6.34-1 tzdata-2010o-1 glibc-2.12.1-4 expat-2.0.1-5 bzip2-1.0.6-1 ncurses-5.7-4 readline-6.1.002-1 bash-4.1.009-1 gdbm-1.8.3-7 gcc-libs-4.5.1-1 db-5.1.19-3 zlib-1.2.5-2 cracklib-2.8.16-1 pam-1.1.1-2 shadow-4.1.4.2-3 attr-2.4.44-1 acl-2.2.49-1 gmp-5.0.1-1 libcap-2.19-1 coreutils-8.7-1 perl-5.12.1-3 openssl-1.0.0.a-3 python-3.1.2-2 nano-2.2.5-1 Total Download Size: 0.00 MB Total Installed Size: 217.92 MB Proceed with installation? [Y/n] y ... debug: partition /, needed 54492, free 20766 debug: returning error 7 from _alpm_check_diskspace : not enough disk space error: not enough free disk space Errors occurred, no packages were upgraded. This is on reiser4 with the lzo compression plugin. Cheers Bryce -- Please note this email and any attachments are intended exclusively for the use of the company or individual shown as the addressee(s). If you have reason to believe you are not the intended recipient please notify the sender by return email immediately and destroy the message you received without making any copies. Any copying, interference or disclosure of this message is therefore unauthorised and expressly prohibited. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Gibson Consulting Services P/L. We do not warrant that this message is virus free. Please perform your own virus check before opening any attachment.

Nezmer

5:25 p.m.

On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...

On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation). The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well. http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987... http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

Allan McRae

11:36 p.m.

On 17/11/10 03:25, Nezmer wrote:

...

On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...
On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation).

The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well.

http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987... http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

I think that initial "sync" is not needed because du does one anyway. The second one looks interesting so I have bookmarked it to look at later. Allan

Dan McGee

17 Nov 17 Nov

12:06 a.m.

On Tue, Nov 16, 2010 at 5:36 PM, Allan McRae wrote:

...

On 17/11/10 03:25, Nezmer wrote:

...
On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...
On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation).

The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well.

http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987...

http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

I think that initial "sync" is not needed because du does one anyway. The second one looks interesting so I have bookmarked it to look at later.

We've putzed around with this a few times, haven't we? This basically "reverts" this one: commit 149839c5391e9a93465f86dbb8d095a0150d755d Author: Xavier Chantry Date: Mon May 26 23:46:01 2008 +0200 du -b is not available on BSD, use du -k instead. This fixes FS#10459. There is apparently no portable ways to get the apparent size of a file, like du -b does. So the best compromise seems to get the block size in kB, and then convert that to byte so that we keep compatibility. Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee There are three or so file sizes that matter: 1) du -s : what we do now, sums up actual taken space so would be more accurate with many < 4K files. 2) du -sb: what we did before and what is proposed, sums up apparent size, so does not necessarily best represent installed size (either sparse files or many <4K files would throw the number off). The 4K assumption also may not always hold... 3) du --tell-me-how-many-blocks-but-not-compressed : what seems like perhaps the ideal? I'm not sure, but this would basically for file in tree: total += ceil(filesize to 4K) 1 is the most portable; 2 we need different flags all over the place; 3 we definitely don't seem to be able to use exiting tools but this would not be an awful one to write. -Dan

Nezmer

6:13 p.m.

On Tue, Nov 16, 2010 at 06:06:32PM -0600, Dan McGee wrote:

...

On Tue, Nov 16, 2010 at 5:36 PM, Allan McRae wrote:

...
On 17/11/10 03:25, Nezmer wrote:

...
On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...
On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation).

The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well.

http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987...

http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

I think that initial "sync" is not needed because du does one anyway. The second one looks interesting so I have bookmarked it to look at later.

We've putzed around with this a few times, haven't we? This basically "reverts" this one:

commit 149839c5391e9a93465f86dbb8d095a0150d755d Author: Xavier Chantry Date: Mon May 26 23:46:01 2008 +0200

du -b is not available on BSD, use du -k instead.

This fixes FS#10459.

There is apparently no portable ways to get the apparent size of a file, like du -b does. So the best compromise seems to get the block size in kB, and then convert that to byte so that we keep compatibility.

Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee

There are three or so file sizes that matter: 1) du -s : what we do now, sums up actual taken space so would be more accurate with many < 4K files. 2) du -sb: what we did before and what is proposed, sums up apparent size, so does not necessarily best represent installed size (either sparse files or many <4K files would throw the number off). The 4K assumption also may not always hold... 3) du --tell-me-how-many-blocks-but-not-compressed : what seems like perhaps the ideal? I'm not sure, but this would basically for file in tree: total += ceil(filesize to 4K)

1 is the most portable; 2 we need different flags all over the place; 3 we definitely don't seem to be able to use exiting tools but this would not be an awful one to write.

-Dan

Is this 3 ? install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 > 0 )) && (( s++ )) (( $s < 4 )) && (( s=4 )) (( install_size+=${s} )) done Unfortunately, this takes around 39 seconds in a 13085 files package.

Dan McGee

6:15 p.m.

On Wed, Nov 17, 2010 at 12:13 PM, Nezmer wrote:

...

On Tue, Nov 16, 2010 at 06:06:32PM -0600, Dan McGee wrote:

...
On Tue, Nov 16, 2010 at 5:36 PM, Allan McRae wrote:

...
On 17/11/10 03:25, Nezmer wrote:

...
On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...
On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation).

The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well.

http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987...

http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

I think that initial "sync" is not needed because du does one anyway. The second one looks interesting so I have bookmarked it to look at later.

We've putzed around with this a few times, haven't we? This basically "reverts" this one:

commit 149839c5391e9a93465f86dbb8d095a0150d755d Author: Xavier Chantry Date: Mon May 26 23:46:01 2008 +0200

du -b is not available on BSD, use du -k instead.

This fixes FS#10459.

There is apparently no portable ways to get the apparent size of a file, like du -b does. So the best compromise seems to get the block size in kB, and then convert that to byte so that we keep compatibility.

Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee

There are three or so file sizes that matter: 1) du -s : what we do now, sums up actual taken space so would be more accurate with many < 4K files. 2) du -sb: what we did before and what is proposed, sums up apparent size, so does not necessarily best represent installed size (either sparse files or many <4K files would throw the number off). The 4K assumption also may not always hold... 3) du --tell-me-how-many-blocks-but-not-compressed : what seems like perhaps the ideal? I'm not sure, but this would basically for file in tree: total += ceil(filesize to 4K)

1 is the most portable; 2 we need different flags all over the place; 3 we definitely don't seem to be able to use exiting tools but this would not be an awful one to write.

-Dan

Is this 3 ?

install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 > 0 )) && (( s++ )) (( $s < 4 )) && (( s=4 )) (( install_size+=${s} )) done

Unfortunately, this takes around 39 seconds in a 13085 files package.

Sure, but we can write a 25 line C program to do the same thing in about 1% of the time. -Dan

Nezmer

6:22 p.m.

On Wed, Nov 17, 2010 at 08:13:08PM +0200, Nezmer wrote:

...

On Tue, Nov 16, 2010 at 06:06:32PM -0600, Dan McGee wrote:

...
On Tue, Nov 16, 2010 at 5:36 PM, Allan McRae wrote:

...
On 17/11/10 03:25, Nezmer wrote:

...
On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...
On 16/11/10 18:02, Bryce Gibson wrote:

...
Hi Allan, I just wanted to ask, it looks like your patches will make a sync fail if it finds there's not enough space, is that correct? Because I'd suggest a warning may be more appropriate, especially for use cases like compressed filesystems. Cheers Bryce ps. I noticed that space checking can be completely disabled via pacman.conf, which may be seen as a suitable solution for people in these types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation).

The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well.

http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987...

http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

I think that initial "sync" is not needed because du does one anyway. The second one looks interesting so I have bookmarked it to look at later.

We've putzed around with this a few times, haven't we? This basically "reverts" this one:

commit 149839c5391e9a93465f86dbb8d095a0150d755d Author: Xavier Chantry Date: Mon May 26 23:46:01 2008 +0200

du -b is not available on BSD, use du -k instead.

This fixes FS#10459.

There is apparently no portable ways to get the apparent size of a file, like du -b does. So the best compromise seems to get the block size in kB, and then convert that to byte so that we keep compatibility.

Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee

There are three or so file sizes that matter: 1) du -s : what we do now, sums up actual taken space so would be more accurate with many < 4K files. 2) du -sb: what we did before and what is proposed, sums up apparent size, so does not necessarily best represent installed size (either sparse files or many <4K files would throw the number off). The 4K assumption also may not always hold... 3) du --tell-me-how-many-blocks-but-not-compressed : what seems like perhaps the ideal? I'm not sure, but this would basically for file in tree: total += ceil(filesize to 4K)

1 is the most portable; 2 we need different flags all over the place; 3 we definitely don't seem to be able to use exiting tools but this would not be an awful one to write.

-Dan

Is this 3 ?

install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 > 0 )) && (( s++ )) (( $s < 4 )) && (( s=4 )) (( install_size+=${s} )) done

Unfortunately, this takes around 39 seconds in a 13085 files package.

Correction: install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 )) && (( s++ )) (( ${s}%4 )) && (( s+=${s}%4 )) (( install_size+=${s} )) done Takes around 40 seconds in a 13085 files package.

Nezmer

7:29 p.m.

On Wed, Nov 17, 2010 at 08:22:14PM +0200, Nezmer wrote:

...

On Wed, Nov 17, 2010 at 08:13:08PM +0200, Nezmer wrote:

...
On Tue, Nov 16, 2010 at 06:06:32PM -0600, Dan McGee wrote:

...
On Tue, Nov 16, 2010 at 5:36 PM, Allan McRae wrote:

...
On 17/11/10 03:25, Nezmer wrote:

...
On Tue, Nov 16, 2010 at 09:03:37PM +1000, Allan McRae wrote:

...
On 16/11/10 18:02, Bryce Gibson wrote: > > Hi Allan, > I just wanted to ask, it looks like your patches will make a sync fail > if it > finds there's not enough space, is that correct? > Because I'd suggest a warning may be more appropriate, especially for > use > cases like compressed filesystems. > Cheers > Bryce > ps. I noticed that space checking can be completely disabled via > pacman.conf, which may be seen as a suitable solution for people in > these > types of situation.

Disabling checking in pacman.conf is what I would recommend in these sort of cases. These are the sort of things we will only find out after we get some widespread usage of the feature. I would be interested in what the size calculations actually do on compressed filesystems.

Allan

On the topic of FS compression(also FS caching and delayed allocation).

The real issue I had was the calculation of install size when building the package. The calculated size was always *very* small. I addressed this issue with 2 patches. The 1st one (adding sync before running du) didn't fix anything. The 2nd patch worked pretty well.

http://gitorious.org/pacman-bsd/pacman-bsd/commit/8b367d4441ba85b5548285d987...

http://gitorious.org/pacman-bsd/pacman-bsd/commit/cf3341fe591293a493bc925e04...

I think that initial "sync" is not needed because du does one anyway. The second one looks interesting so I have bookmarked it to look at later.

We've putzed around with this a few times, haven't we? This basically "reverts" this one:

commit 149839c5391e9a93465f86dbb8d095a0150d755d Author: Xavier Chantry Date: Mon May 26 23:46:01 2008 +0200

du -b is not available on BSD, use du -k instead.

This fixes FS#10459.

There is apparently no portable ways to get the apparent size of a file, like du -b does. So the best compromise seems to get the block size in kB, and then convert that to byte so that we keep compatibility.

Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee

There are three or so file sizes that matter: 1) du -s : what we do now, sums up actual taken space so would be more accurate with many < 4K files. 2) du -sb: what we did before and what is proposed, sums up apparent size, so does not necessarily best represent installed size (either sparse files or many <4K files would throw the number off). The 4K assumption also may not always hold... 3) du --tell-me-how-many-blocks-but-not-compressed : what seems like perhaps the ideal? I'm not sure, but this would basically for file in tree: total += ceil(filesize to 4K)

1 is the most portable; 2 we need different flags all over the place; 3 we definitely don't seem to be able to use exiting tools but this would not be an awful one to write.

-Dan

Is this 3 ?

install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 > 0 )) && (( s++ )) (( $s < 4 )) && (( s=4 )) (( install_size+=${s} )) done

Unfortunately, this takes around 39 seconds in a 13085 files package.

Correction:

install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 )) && (( s++ )) (( ${s}%4 )) && (( s+=${s}%4 )) (( install_size+=${s} )) done

Takes around 40 seconds in a 13085 files package.

Fail As Dan said, This is too slow to be done in bash. I just want to correct it for reference. install_size=0 for f in `find $pkgdir -type f`; do (( s=$(${SIZECMD} ${f})/1024 )) (( $(${SIZECMD} ${f})%1024 )) && (( s++ )) (( ${s}%4 )) && (( s+=4-${s}%4 )) (( install_size+=${s} )) done

Xavier Chantry

8:37 p.m.

On Wed, Nov 17, 2010 at 1:06 AM, Dan McGee wrote:

...

We've putzed around with this a few times, haven't we? This basically "reverts" this one:

commit 149839c5391e9a93465f86dbb8d095a0150d755d Author: Xavier Chantry Date: Mon May 26 23:46:01 2008 +0200

du -b is not available on BSD, use du -k instead.

This fixes FS#10459.

There is apparently no portable ways to get the apparent size of a file, like du -b does. So the best compromise seems to get the block size in kB, and then convert that to byte so that we keep compatibility.

Signed-off-by: Xavier Chantry Signed-off-by: Dan McGee

It doesn't revert my commit, it just takes the alternative approach which was already suggested in the bug report : https://bugs.archlinux.org/task/10459#comment28616 We do this already for stat and sed, we do this in C (c.f. diskspace), so why not for du ? I've no problem with that. That said, I've no problem with a C impl either. Would it be portable ? Just wanted to mention that even though I wrote this patch, I am for restoring apparent size. IIRC we already got a report/request for that a while ago and I already felt that way back then.

Dan McGee

2:12 a.m.

New subject: [pacman-dev] [PATCH 1/2] Refactor statfs/statvfs type check

Turn it into a configure-type typedef, which allows us to reduce the amount of duplicated code and clean up some #ifdef magic in the code itself. Adjust some of the other defined checks to look at the headers available rather than trying to pull in the right ones based on configure checks. Signed-off-by: Dan McGee --- acinclude.m4 | 28 ++++++++++++++++++++++++- configure.ac | 22 ++----------------- lib/libalpm/diskspace.c | 51 +++++++++++++++------------------------------- lib/libalpm/diskspace.h | 11 +++------ 4 files changed, 50 insertions(+), 62 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index 740d9ec..6693da4 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -77,7 +77,7 @@ AC_DEFUN([GCC_VISIBILITY_CC],[ if test $visibility_cv_cc = yes; then AC_DEFINE([ENABLE_VISIBILITY_CC], 1, [Define if symbol visibility C support is enabled.]) fi - AM_CONDITIONAL([ENABLE_VISIBILITY_CC], test "x$visibility_cv_cc" = "xyes") + AM_CONDITIONAL([ENABLE_VISIBILITY_CC], test "x$visibility_cv_cc" = "xyes") fi ]) @@ -97,7 +97,31 @@ AC_DEFUN([GCC_GNU89_INLINE_CC],[ if test $gnu89_inline_cv_cc = yes; then AC_DEFINE([ENABLE_GNU89_INLINE_CC], 1, [Define if gnu89 inlining semantics should be used.]) fi - AM_CONDITIONAL([ENABLE_GNU89_INLINE_CC], test "x$gnu89_inline_cv_cc" = "xyes") + AM_CONDITIONAL([ENABLE_GNU89_INLINE_CC], test "x$gnu89_inline_cv_cc" = "xyes") fi ]) +dnl Checks for getmntinfo and determines whether it uses statfs or statvfs +AC_DEFUN([FS_STATS_TYPE], + [AC_CACHE_CHECK([filesystem statistics type], fs_stats_cv_type, + [AC_CHECK_FUNC(getmntinfo, + [AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[ +# include +# include +#if HAVE_SYS_UCRED_H +#include +#endif +extern int getmntinfo (struct statfs **, int); +]], + [])], + [fs_stats_cv_type="struct statfs"], + [fs_stats_cv_type="struct statvfs"])], + [AC_CHECK_FUNC(getmntent, + [fs_stats_cv_type="struct statvfs"])] + )] + ) + AC_DEFINE_UNQUOTED(FSSTATSTYPE, [$fs_stats_cv_type], + [Defined as the filesystem stats type ('statvfs' or 'statfs')]) +]) + diff --git a/configure.ac b/configure.ac index b9e229d..22528ce 100644 --- a/configure.ac +++ b/configure.ac @@ -191,27 +191,11 @@ AC_FUNC_GETMNTENT AC_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK AC_FUNC_MKTIME AC_TYPE_SIGNAL -AC_CHECK_FUNCS([geteuid realpath regcomp strcasecmp \ +AC_CHECK_FUNCS([geteuid getmntinfo realpath regcomp strcasecmp \ strndup strrchr strsep swprintf \ wcwidth uname]) - -# Checks for getmntinfo and determines whether it uses statfs or statvfs -AC_CHECK_FUNC(getmntinfo, - [AC_MSG_CHECKING(parameter style for getmntinfo) - AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ -# include -# include -#if HAVE_SYS_UCRED_H -#include -#endif -extern int getmntinfo (struct statfs **, int); - ]], [])], - [AC_DEFINE(HAVE_GETMNTINFO_STATFS, [], [getmntinfo uses statfs]) - AC_MSG_RESULT(statfs)], - [AC_DEFINE(HAVE_GETMNTINFO_STATVFS, [], [getmntinfo uses statvfs]) - AC_MSG_RESULT(statvfs)]) - ]) - +# For the diskspace code +FS_STATS_TYPE # Enable large file support if available AC_SYS_LARGEFILE diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index 1f1a620..ad6ceba 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -19,18 +19,23 @@ #include "config.h" -#if defined HAVE_GETMNTENT +#if defined(HAVE_MNTENT_H) #include +#endif +#if defined(HAVE_SYS_STATVFS_H) #include -#elif defined HAVE_GETMNTINFO_STATFS +#endif +#if defined(HAVE_SYS_PARAM_H) #include +#endif +#if defined(HAVE_SYS_MOUNT_H) #include -#if HAVE_SYS_UCRED_H +#endif +#if defined(HAVE_SYS_UCRED_H) #include #endif -#elif defined HAVE_GETMNTINFO_STATVFS +#if defined(HAVE_SYS_TYPES_H) #include -#include #endif #include @@ -60,7 +65,7 @@ static alpm_list_t *mount_point_list() #if defined HAVE_GETMNTENT struct mntent *mnt; FILE *fp; - struct statvfs fsp; + FSSTATSTYPE fsp; fp = setmntent(MOUNTED, "r"); @@ -77,8 +82,8 @@ static alpm_list_t *mount_point_list() MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_dir); - MALLOC(mp->fsp, sizeof(struct statvfs), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)(&fsp), sizeof(struct statvfs)); + MALLOC(mp->fsp, sizeof(FSSTATSTYPE), RET_ERR(PM_ERR_MEMORY, NULL)); + memcpy((void *)(mp->fsp), (void *)(&fsp), sizeof(FSSTATSTYPE)); mp->blocks_needed = 0; mp->max_blocks_needed = 0; @@ -88,9 +93,9 @@ static alpm_list_t *mount_point_list() } endmntent(fp); -#elif defined HAVE_GETMNTINFO_STATFS +#elif defined HAVE_GETMNTINFO int entries; - struct statfs *fsp; + FSSTATSTYPE *fsp; entries = getmntinfo(&fsp, MNT_NOWAIT); @@ -102,30 +107,8 @@ static alpm_list_t *mount_point_list() MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(fsp->f_mntonname); - MALLOC(mp->fsp, sizeof(struct statfs), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)fsp, sizeof(struct statfs)); - - mp->blocks_needed = 0; - mp->max_blocks_needed = 0; - - mount_points = alpm_list_add(mount_points, mp); - } -#elif defined HAVE_GETMNTINFO_STATVFS - int entries; - struct statvfs *fsp; - - entries = getmntinfo(&fsp, MNT_NOWAIT); - - if (entries < 0) { - return NULL; - } - - for (; entries-- > 0; fsp++) { - MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); - mp->mount_dir = strdup(fsp->f_mntonname); - - MALLOC(mp->fsp, sizeof(struct statvfs), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)fsp, sizeof(struct statvfs)); + MALLOC(mp->fsp, sizeof(FSSTATSTYPE), RET_ERR(PM_ERR_MEMORY, NULL)); + memcpy((void *)(mp->fsp), (void *)fsp, sizeof(FSSTATSTYPE)); mp->blocks_needed = 0; mp->max_blocks_needed = 0; diff --git a/lib/libalpm/diskspace.h b/lib/libalpm/diskspace.h index 15ff2f6..60c0028 100644 --- a/lib/libalpm/diskspace.h +++ b/lib/libalpm/diskspace.h @@ -20,9 +20,10 @@ #ifndef _ALPM_DISKSPACE_H #define _ALPM_DISKSPACE_H -#if defined HAVE_GETMNTINFO_STATFS +#if defined(HAVE_SYS_MOUNT_H) #include -#else +#endif +#if defined(HAVE_SYS_STATVFS_H) #include #endif @@ -31,15 +32,11 @@ typedef struct __alpm_mountpoint_t { /* mount point information */ char *mount_dir; -#if defined HAVE_GETMNTINFO_STATFS - struct statfs *fsp; -#else - struct statvfs *fsp; -#endif /* storage for additional disk usage calculations */ long blocks_needed; long max_blocks_needed; int used; + FSSTATSTYPE *fsp; } alpm_mountpoint_t; int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db); -- 1.7.3.2

Dan McGee

2:12 a.m.

New subject: [pacman-dev] [PATCH 2/2] diskspace style cleanups and small fixes

* Use our normal return() function syntax * Rework a few things to reduce number of casts * Fix void function argument declaration * Add missing gettext _() call * Remove need for seperate malloc() of statvfs/statfs structure * Unify argument order of static functions- mountpoints now always passed first * Count all files that start with '.' in a package against the DB * Rename db to db_local in check_diskspace to clarify some code * Fix some line wrapping to respect 80 characters Signed-off-by: Dan McGee --- lib/libalpm/diskspace.c | 101 ++++++++++++++++++++++++---------------------- lib/libalpm/diskspace.h | 4 +- 2 files changed, 55 insertions(+), 50 deletions(-) diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index ad6ceba..a96228a 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c @@ -52,12 +52,14 @@ #include "trans.h" #include "handle.h" -static int mount_point_cmp(const alpm_mountpoint_t *mp1, const alpm_mountpoint_t *mp2) +static int mount_point_cmp(const void *p1, const void *p2) { + const alpm_mountpoint_t *mp1 = p1; + const alpm_mountpoint_t *mp2 = p2; return(strcmp(mp1->mount_dir, mp2->mount_dir)); } -static alpm_list_t *mount_point_list() +static alpm_list_t *mount_point_list(void) { alpm_list_t *mount_points = NULL; alpm_mountpoint_t *mp; @@ -70,20 +72,19 @@ static alpm_list_t *mount_point_list() fp = setmntent(MOUNTED, "r"); if (fp == NULL) { - return NULL; + return(NULL); } - while((mnt = getmntent (fp))) { + while((mnt = getmntent(fp))) { if(statvfs(mnt->mnt_dir, &fsp) != 0) { - _alpm_log(PM_LOG_WARNING, "could not get filesystem information for %s\n", mnt->mnt_dir); + _alpm_log(PM_LOG_WARNING, + _("could not get filesystem information for %s\n"), mnt->mnt_dir); continue; } MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_dir); - - MALLOC(mp->fsp, sizeof(FSSTATSTYPE), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)(&fsp), sizeof(FSSTATSTYPE)); + memcpy(&(mp->fsp), &fsp, sizeof(FSSTATSTYPE)); mp->blocks_needed = 0; mp->max_blocks_needed = 0; @@ -106,9 +107,7 @@ static alpm_list_t *mount_point_list() for(; entries-- > 0; fsp++) { MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(fsp->f_mntonname); - - MALLOC(mp->fsp, sizeof(FSSTATSTYPE), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)fsp, sizeof(FSSTATSTYPE)); + memcpy(&(mp->fsp), &fsp, sizeof(FSSTATSTYPE)); mp->blocks_needed = 0; mp->max_blocks_needed = 0; @@ -118,11 +117,12 @@ static alpm_list_t *mount_point_list() #endif mount_points = alpm_list_msort(mount_points, alpm_list_count(mount_points), - (alpm_list_fn_cmp)mount_point_cmp); + mount_point_cmp); return(mount_points); } -static alpm_list_t *match_mount_point(const alpm_list_t *mount_points, const char *file) +static alpm_list_t *match_mount_point(const alpm_list_t *mount_points, + const char *file) { char real_path[PATH_MAX]; snprintf(real_path, PATH_MAX, "%s%s", handle->root, file); @@ -132,17 +132,18 @@ static alpm_list_t *match_mount_point(const alpm_list_t *mount_points, const cha alpm_mountpoint_t *data = mp->data; if(strncmp(data->mount_dir, real_path, strlen(data->mount_dir)) == 0) { - return mp; + return(mp); } mp = mp->prev; } while (mp != alpm_list_last(mount_points)); /* should not get here... */ - return NULL; + return(NULL); } -static int calculate_removed_size(pmpkg_t *pkg, const alpm_list_t *mount_points) +static int calculate_removed_size(const alpm_list_t *mount_points, + pmpkg_t *pkg) { alpm_list_t *file; @@ -152,37 +153,41 @@ static int calculate_removed_size(pmpkg_t *pkg, const alpm_list_t *mount_points) alpm_mountpoint_t *data; struct stat st; char path[PATH_MAX]; + const char *filename = file->data; - /* skip directories to be consistent with libarchive that reports them to be zero size - and to prevent multiple counting across packages */ - if(*((char *)(file->data) + strlen(file->data) - 1) == '/') { + /* skip directories to be consistent with libarchive that reports them + * to be zero size and to prevent multiple counting across packages */ + if(*(filename + strlen(filename) - 1) == '/') { continue; } - mp = match_mount_point(mount_points, file->data); + mp = match_mount_point(mount_points, filename); if(mp == NULL) { - _alpm_log(PM_LOG_WARNING, _("could not determine mount point for file %s"), (char *)(file->data)); + _alpm_log(PM_LOG_WARNING, + _("could not determine mount point for file %s"), filename); continue; } - snprintf(path, PATH_MAX, "%s%s", handle->root, (char *)file->data); + snprintf(path, PATH_MAX, "%s%s", handle->root, filename); _alpm_lstat(path, &st); - /* skip symlinks to be consistent with libarchive that reports them to be zero size */ + /* skip symlinks to be consistent with libarchive that reports them to + * be zero size */ if(S_ISLNK(st.st_mode)) { continue; } data = mp->data; data->blocks_needed -= ceil((double)(st.st_size) / - (double)(data->fsp->f_bsize)); + (double)(data->fsp.f_bsize)); data->used = 1; } - return 0; + return(0); } -static int calculate_installed_size(pmpkg_t *pkg, const alpm_list_t *mount_points) +static int calculate_installed_size(const alpm_list_t *mount_points, + pmpkg_t *pkg) { int ret=0; struct archive *archive; @@ -212,31 +217,30 @@ static int calculate_installed_size(pmpkg_t *pkg, const alpm_list_t *mount_point file = archive_entry_pathname(entry); /* approximate space requirements for db entries */ - if(strcmp(file, ".PKGINFO") == 0 || - strcmp(file, ".INSTALL") == 0 || - strcmp(file, ".CHANGELOG") == 0) { + if(file[0] == '.') { file = alpm_option_get_dbpath(); } mp = match_mount_point(mount_points, file); if(mp == NULL) { - _alpm_log(PM_LOG_WARNING, _("could not determine mount point for file %s"), archive_entry_pathname(entry)); + _alpm_log(PM_LOG_WARNING, + _("could not determine mount point for file %s"), file); continue; } data = mp->data; data->blocks_needed += ceil((double)(archive_entry_size(entry)) / - (double)(data->fsp->f_bsize)); + (double)(data->fsp.f_bsize)); data->used = 1; } archive_read_finish(archive); cleanup: - return ret; + return(ret); } -int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db) +int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db_local) { alpm_list_t *mount_points, *i; int replaces = 0, abort = 0; @@ -247,8 +251,8 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db) mount_points = mount_point_list(); if(mount_points == NULL) { - _alpm_log(PM_LOG_ERROR, _("count not determine filesystem mount points")); - return -1; + _alpm_log(PM_LOG_ERROR, _("could not determine filesystem mount points")); + return(-1); } replaces = alpm_list_count(trans->remove); @@ -257,23 +261,24 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db) for(targ = trans->remove; targ; targ = targ->next, current++) { double percent = (double)current / numtargs; PROGRESS(trans, PM_TRANS_PROGRESS_DISKSPACE_START, "", (percent * 100), - numtargs, current); + numtargs, current); - pkg = (pmpkg_t*)targ->data; - calculate_removed_size(pkg, mount_points); + pkg = targ->data; + calculate_removed_size(mount_points, pkg); } } for(targ = trans->add; targ; targ = targ->next, current++) { double percent = (double)current / numtargs; PROGRESS(trans, PM_TRANS_PROGRESS_DISKSPACE_START, "", (percent * 100), - numtargs, current); + numtargs, current); - pkg = (pmpkg_t*)targ->data; - if(_alpm_db_get_pkgfromcache(db, pkg->name)) { - calculate_removed_size(pkg, mount_points); + pkg = targ->data; + /* is this package already installed? */ + if(_alpm_db_get_pkgfromcache(db_local, pkg->name)) { + calculate_removed_size(mount_points, pkg); } - calculate_installed_size(pkg, mount_points); + calculate_installed_size(mount_points, pkg); for(i = mount_points; i; i = alpm_list_next(i)) { alpm_mountpoint_t *data = i->data; @@ -284,14 +289,15 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db) } PROGRESS(trans, PM_TRANS_PROGRESS_DISKSPACE_START, "", 100, - numtargs, current); + numtargs, current); for(i = mount_points; i; i = alpm_list_next(i)) { alpm_mountpoint_t *data = i->data; if(data->used == 1) { _alpm_log(PM_LOG_DEBUG, "partition %s, needed %ld, free %ld\n", - data->mount_dir, data->max_blocks_needed, (long int)(data->fsp->f_bfree)); - if(data->max_blocks_needed > data->fsp->f_bfree) { + data->mount_dir, data->max_blocks_needed, + (unsigned long)data->fsp.f_bfree); + if(data->max_blocks_needed > data->fsp.f_bfree) { abort = 1; } } @@ -300,7 +306,6 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db) for(i = mount_points; i; i = alpm_list_next(i)) { alpm_mountpoint_t *data = i->data; FREE(data->mount_dir); - FREE(data->fsp); } FREELIST(mount_points); @@ -308,7 +313,7 @@ int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db) RET_ERR(PM_ERR_DISK_SPACE, -1); } - return 0; + return(0); } /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/diskspace.h b/lib/libalpm/diskspace.h index 60c0028..e73497e 100644 --- a/lib/libalpm/diskspace.h +++ b/lib/libalpm/diskspace.h @@ -36,10 +36,10 @@ typedef struct __alpm_mountpoint_t { long blocks_needed; long max_blocks_needed; int used; - FSSTATSTYPE *fsp; + FSSTATSTYPE fsp; } alpm_mountpoint_t; -int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db); +int _alpm_check_diskspace(pmtrans_t *trans, pmdb_t *db_local); #endif /* _ALPM_DISKSPACE_H */ -- 1.7.3.2

Allan McRae

6:08 a.m.

New subject: [pacman-dev] [PATCH 2/2] diskspace style cleanups and small fixes

On 17/11/10 12:12, Dan McGee wrote:

...

* Use our normal return() function syntax * Rework a few things to reduce number of casts * Fix void function argument declaration * Add missing gettext _() call * Remove need for seperate malloc() of statvfs/statfs structure * Unify argument order of static functions- mountpoints now always passed first * Count all files that start with '.' in a package against the DB * Rename db to db_local in check_diskspace to clarify some code * Fix some line wrapping to respect 80 characters

Signed-off-by: Dan McGee

(after minor fix below) Signed-off-by: Allan

...

--- lib/libalpm/diskspace.c | 101 ++++++++++++++++++++++++---------------------- lib/libalpm/diskspace.h | 4 +- 2 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/lib/libalpm/diskspace.c b/lib/libalpm/diskspace.c index ad6ceba..a96228a 100644 --- a/lib/libalpm/diskspace.c +++ b/lib/libalpm/diskspace.c ... @@ -70,20 +72,19 @@ static alpm_list_t *mount_point_list() fp = setmntent(MOUNTED, "r");

if (fp == NULL) { - return NULL; + return(NULL); }

- while((mnt = getmntent (fp))) { + while((mnt = getmntent(fp))) { if(statvfs(mnt->mnt_dir,&fsp) != 0) { - _alpm_log(PM_LOG_WARNING, "could not get filesystem information for %s\n", mnt->mnt_dir); + _alpm_log(PM_LOG_WARNING, + _("could not get filesystem information for %s\n"), mnt->mnt_dir); continue; }

MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(mnt->mnt_dir); - - MALLOC(mp->fsp, sizeof(FSSTATSTYPE), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)(&fsp), sizeof(FSSTATSTYPE)); + memcpy(&(mp->fsp),&fsp, sizeof(FSSTATSTYPE));

mp->blocks_needed = 0; mp->max_blocks_needed = 0; @@ -106,9 +107,7 @@ static alpm_list_t *mount_point_list() for(; entries--> 0; fsp++) { MALLOC(mp, sizeof(alpm_mountpoint_t), RET_ERR(PM_ERR_MEMORY, NULL)); mp->mount_dir = strdup(fsp->f_mntonname); - - MALLOC(mp->fsp, sizeof(FSSTATSTYPE), RET_ERR(PM_ERR_MEMORY, NULL)); - memcpy((void *)(mp->fsp), (void *)fsp, sizeof(FSSTATSTYPE)); + memcpy(&(mp->fsp),&fsp, sizeof(FSSTATSTYPE));

fsp is a pointer in the getmntinfo code path so... memcpy(&(mp->fsp), fsp, sizeof(FSSTATSTYPE)); Allan

4910

Age (days ago)

4912

Last active (days ago)

List overview

Download

16 comments

6 participants

participants (6)

Allan McRae
Bryce Gibson
Dan McGee
Dan McGee
Nezmer
Xavier Chantry