Skip to content

Commit

Permalink
Improve the overflow fix
Browse files Browse the repository at this point in the history
  • Loading branch information
lslezak committed Jul 3, 2018
1 parent 2e3bcf6 commit cbd2188
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 115 deletions.
199 changes: 84 additions & 115 deletions src/NCPkgPopupDiskspace.cc
Expand Up @@ -59,6 +59,9 @@

#include "NCi18n.h"

// zypp::str::form()
#include <zypp/base/String.h>

// set values as set in YQPkgDiskUsageList.cc
#define MIN_FREE_MB_WARN 400
#define MIN_FREE_MB_PROXIMITY 700
Expand Down Expand Up @@ -108,52 +111,42 @@ NCPkgDiskspace::~NCPkgDiskspace()
{
}


namespace
{

// Some partitions are skipped for the popup.

bool
skip_partition( const ZyppPartitionDu & partition )
namespace {
std::string formatSize(unsigned long long size, int width = 0)
{
if (partition.readonly)
{
yuiMilestone() << "skipping " << partition.dir << " due to read-only flag" << endl;
return true;
}

// Workaround for partitions equal or larger than 8 EiB (see bug
// #991090).

long long used_size = partition.used_size * FSize::KB; // before package installation
long long pkg_size = partition.pkg_size * FSize::KB; // after package installation
long long total_size = partition.total_size * FSize::KB;
// FSize::bestUnit does not work for huge numbers so only use it for small once
FSize::Unit unit = (size >= FSize::TB) ? FSize::T : FSize(size).bestUnit();
int prec = unit == FSize::B ? 0 : 2;

// Skip partition if the used size after package installation is
// smaller or equal to the used size before package installation.

if (pkg_size <= used_size)
{
yuiMilestone() << "skipping " << partition.dir << " due to not being used" << endl;
return true;
}
return zypp::str::form( "%*.*f %s", width, prec, double(size) / FSize::factor(unit), FSize::unit(unit));
}

// Skip partition if free size before package installation is negative
// since that indicates overflows.
/**
* Compute percent usage
* @param used used size (any unit, but the same as the "total")
* @param total total size (any unit, but the same as the "used")
* @return percent, might be more than 100 if the size of the selected
* packages is bigger than the available free size
*/
int usedPercentInt(long long used, long long total)
{
int percent = 0;

if (total_size - used_size < 0)
{
yuiMilestone() << "skipping " << partition.dir << " due to negative value" << endl;
return true;
}
if ( total != 0 )
// use double to avoid overflow
percent = ( 100.0 * used ) / total;

return false;
return percent;
}

// see usedPercentInt()
std::string usedPercentStr(long long used, long long total)
{
int percent = usedPercentInt(used, total);
return zypp::str::form( "%2d%%", percent );
}
}


///////////////////////////////////////////////////////////////////
//
//
Expand All @@ -167,45 +160,37 @@ void NCPkgDiskspace::fillPartitionTable()
NCTable * partitions = popupWin->Partitions();
partitions->deleteAllItems(); // clear table

YTableItem * newItem;

zypp::ZYpp::Ptr z = zypp::getZYpp();
ZyppDuSet du = z->diskUsage ();
ZyppDuSetIterator
b = du.begin (),
e = du.end ();

if (b == e)
if (du.empty())
{
// retry after detecting from the target
z->setPartitions(zypp::DiskUsageCounter::detectMountPoints ());
du = z->diskUsage();
b = du.begin ();
e = du.end ();
// retry after detecting from the target
z->setPartitions(zypp::DiskUsageCounter::detectMountPoints ());
du = z->diskUsage();
}

for (ZyppDuSetIterator it = b; it != e; ++it)
for (const ZyppPartitionDu &item: du)
{
if (skip_partition (*it))
continue;
if (item.readonly)
continue;

zypp::ByteCount pkg_used (it->pkg_size * 1024);
unsigned long long pkg_used = item.pkg_size * FSize::KB;
// still not perfect, this might underflow if the size of selected
// packages to install is bigger than the total disk size :-(
unsigned long long pkg_available = (item.total_size - item.pkg_size) * FSize::KB;
unsigned long long total = item.total_size * FSize::KB;

zypp::ByteCount pkg_available ((it->total_size - it->pkg_size) * 1024);

zypp::ByteCount total (it->total_size * 1024);

newItem = new YTableItem( it->dir,
pkg_used.asString (8),
pkg_available.asString (8),
total.asString (8),
usedPercent( it->pkg_size, it->total_size ) );
YTableItem * newItem = new YTableItem( item.dir,
formatSize(pkg_used, 8),
formatSize(pkg_available, 8),
formatSize(total, 8),
usedPercentStr( item.pkg_size, item.total_size ) );

partitions->addItem( newItem );
}
}


///////////////////////////////////////////////////////////////////
//
//
Expand All @@ -217,39 +202,35 @@ void NCPkgDiskspace::fillPartitionTable()
//
std::string NCPkgDiskspace::checkDiskSpace()
{
std::string text = "";
std::string text;

zypp::ZYpp::Ptr z = zypp::getZYpp();
ZyppDuSet du = z->diskUsage ();
ZyppDuSetIterator
b = du.begin (),
e = du.end (),
it;
if (b == e)

if (du.empty())
{
// retry after detecting from the target
z->setPartitions(zypp::DiskUsageCounter::detectMountPoints ());
du = z->diskUsage();
b = du.begin ();
e = du.end ();
// retry after detecting from the target
z->setPartitions(zypp::DiskUsageCounter::detectMountPoints ());
du = z->diskUsage();
}

for (it = b; it != e; ++it)
for (const ZyppPartitionDu &item: du)
{
if (skip_partition (*it))
if (item.readonly)
continue;

zypp::ByteCount pkg_available = (it->total_size - it->pkg_size) * 1024;
// available size (in KiB!)
long long pkg_available = item.total_size - item.pkg_size;
if ( pkg_available < 0 )
{
text += "\"";
text += it->dir;
text += item.dir;
text += "\"";
text += " ";
text += NCPkgStrings::MoreText();
text += " ";
std::string available = pkg_available.asString();
text += available.replace( 0, 1, " " ); // clear the minus sign??
// convert to a positive number before formatting
text += formatSize(-pkg_available * FSize::KB);
text += " ";
text += NCPkgStrings::MoreSpaceText();
text += "<br>";
Expand All @@ -269,32 +250,30 @@ std::string NCPkgDiskspace::checkDiskSpace()
//
void NCPkgDiskspace::checkRemainingDiskSpace( const ZyppPartitionDu & partition )
{
if ( skip_partition ( partition ) )
if ( partition.readonly )
return;

FSize usedSize ( partition.pkg_size, FSize::K );
FSize totalSize ( partition.total_size, FSize::K );
int percent = usedPercentInt(partition.pkg_size, partition.total_size);

int percent = 0;

if ( totalSize != 0 )
percent = ( 100 * usedSize ) / totalSize;
// free in MiB - libzyp sizes are already in KiB, divide by 1024 to get MiB
long long free = (partition.total_size - partition.pkg_size) / 1024;

int free = ( totalSize - usedSize ) / FSize::MB;

yuiMilestone() << "Partition: " << partition.dir << " Used percent: "
<< percent << " Free: " << free << endl;
yuiMilestone() << "Partition: " << partition.dir << " Total (MiB): "
<< partition.total_size / 1024 << " Used (MiB): " << partition.pkg_size / 1024
<< " Used percent: " << percent << "% Free (MiB): " << free << endl;

if ( percent > MIN_PERCENT_WARN )
{
// Modern hard disks can be huge, so a warning based on percentage only
// can be misleading - check the absolute value, too.
if ( free < MIN_FREE_MB_PROXIMITY )
{
yuiWarning() << "free < MIN_FREE_MB_PROXIMITY (" << MIN_FREE_MB_PROXIMITY << ")" << endl;
runningOutWarning.enterProximity();
}
if ( free < MIN_FREE_MB_WARN )
{
yuiWarning() << "free < MIN_FREE_MB_WARN (" << MIN_FREE_MB_WARN << ")" << endl;
runningOutWarning.enterRange();
}
}
Expand Down Expand Up @@ -327,20 +306,14 @@ void NCPkgDiskspace::checkRemainingDiskSpace( const ZyppPartitionDu & partition
//
void NCPkgDiskspace::setDiskSpace( wint_t ch )
{
int percent = 0;

// set diskspace values in ZyppDuSet testDiskSpace
for ( ZyppDuSetIterator it = testDiskUsage.begin();
it != testDiskUsage.end();
++it )
{
const ZyppPartitionDu & partitionDu = *it;

FSize usedSize ( partitionDu.pkg_size, FSize::K );
FSize totalSize ( partitionDu.total_size, FSize::K );

if ( totalSize != 0 )
percent = ( 100 * usedSize ) / totalSize;
int percent = usedPercentInt(partitionDu.pkg_size, partitionDu.total_size);

if ( ch == '+' )
percent += 3;
Expand All @@ -350,12 +323,11 @@ void NCPkgDiskspace::setDiskSpace( wint_t ch )
if ( percent < 0 )
percent = 0;

partitionDu.pkg_size = partitionDu.total_size * percent / 100;
partitionDu.pkg_size = partitionDu.total_size / 100 * percent;

FSize newSize ( partitionDu.pkg_size, FSize::K );

yuiMilestone() << "Used size (MB): " << newSize / FSize::MB << endl;
yuiMilestone() << "Total size (MB): " << totalSize / FSize::MB << endl;
// libzyp sizes are already in KiB, divide by 1024 to get MiB
yuiMilestone() << "Used size (MiB): " << partitionDu.pkg_size / 1024 << endl;
yuiMilestone() << "Total size (MiB): " << partitionDu.total_size / 1024 << endl;
}
}

Expand Down Expand Up @@ -421,13 +393,14 @@ void NCPkgDiskspace::checkDiskSpaceRange( )
}
}

// FIXME: do not use this, contains overflow bug, use usedPercentStr() instead!
std::string NCPkgDiskspace::usedPercent( FSize used, FSize total )
{
int percent = 0;
char percentStr[10];

if ( total != 0 )
percent = ( 100 * used ) / total;
percent = ( 100 * used ) / total;

sprintf( percentStr, "%d%%", percent );

Expand Down Expand Up @@ -456,23 +429,19 @@ zypp::ByteCount NCPkgDiskspace::calculateDiff()
{
zypp::ZYpp::Ptr z = zypp::getZYpp();
ZyppDuSet du = z->diskUsage ();
ZyppDuSetIterator
b = du.begin (),
e = du.end (),
it;
if (b == e)

if (du.empty())
{
// retry after detecting from the target
z->setPartitions(zypp::DiskUsageCounter::detectMountPoints ());
du = z->diskUsage();
b = du.begin ();
e = du.end ();
// retry after detecting from the target
z->setPartitions(zypp::DiskUsageCounter::detectMountPoints ());
du = z->diskUsage();
}

zypp::ByteCount diff = 0;
for (it = b; it != e; ++it)
for (const ZyppPartitionDu &item: du)
{
diff += (it->pkg_size - it->used_size) * 1024;
// the diff should be normally very small, the "long long" limit should never be reached (TM)
diff += (item.pkg_size - item.used_size) * 1024;
}

return diff;
Expand Down
1 change: 1 addition & 0 deletions src/NCPkgPopupDiskspace.h
Expand Up @@ -204,6 +204,7 @@ class NCPkgDiskspace {
NCPkgPopupDiskspace *popupWin;
ZyppDuSet testDiskUsage;

// FIXME: not used anymore, might overflow
std::string usedPercent( FSize used, FSize total );

/**
Expand Down

0 comments on commit cbd2188

Please sign in to comment.