From dga@sheep.lcs.mit.edu Tue Jun 1 15:38:17 2004 Return-Path: Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 060C216A4CE for ; Tue, 1 Jun 2004 15:38:17 -0700 (PDT) Received: from sheep.lcs.mit.edu (sheep.lcs.mit.edu [18.31.0.136]) by mx1.FreeBSD.org (Postfix) with ESMTP id B4B7543D45 for ; Tue, 1 Jun 2004 15:38:16 -0700 (PDT) (envelope-from dga@sheep.lcs.mit.edu) Received: from sheep.lcs.mit.edu (localhost [127.0.0.1]) by sheep.lcs.mit.edu (8.12.10/8.12.10) with ESMTP id i51McFgi000809 for ; Tue, 1 Jun 2004 18:38:15 -0400 (EDT) (envelope-from dga@sheep.lcs.mit.edu) Received: (from root@localhost) by sheep.lcs.mit.edu (8.12.10/8.12.10/Submit) id i51McFGw000808; Tue, 1 Jun 2004 18:38:15 -0400 (EDT) (envelope-from dga) Message-Id: <200406012238.i51McFGw000808@sheep.lcs.mit.edu> Date: Tue, 1 Jun 2004 18:38:15 -0400 (EDT) From: David Andersen Reply-To: David Andersen To: FreeBSD-gnats-submit@freebsd.org Cc: Subject: df -m and -g incorrect with negative avail X-Send-Pr-Version: 3.113 X-GNATS-Notify: >Number: 67467 >Category: bin >Synopsis: df -m and -g incorrect with negative avail >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Jun 01 15:40:07 PDT 2004 >Closed-Date: Fri Jun 04 02:31:31 PDT 2004 >Last-Modified: Fri Jun 04 02:31:31 PDT 2004 >Originator: David Andersen >Release: FreeBSD 5.2.1-RELEASE i386 >Organization: MIT >Environment: System: FreeBSD sheep.lcs.mit.edu 5.2.1-RELEASE FreeBSD 5.2.1-RELEASE #0: Mon Feb 23 20:45:55 GMT 2004 root@wv1u.btc.adaptec.com:/usr/obj/usr/src/sys/GENERIC i386 >Description: df using the -m or -g flags appears to be using an unsigned quantity to track the amount of available disk space. This should be a signed quantity, as available disk space can be negative. The -k flag handles negative disk space properly: 503 sheep:~> df -k /ron Filesystem 1K-blocks Used Avail Capacity Mounted on /dev/ar0s1e 315131888 303691232 -13769892 105% /ron 502 sheep:~> df -g /ron Filesystem 1G-blocks Used Avail Capacity Mounted on /dev/ar0s1e 300 289 70368744177650 105% /ron >How-To-Repeat: Fill a disk over 100%, and run df. >Fix: Sorry, don't have the time to fix right now, may append a fix later. >Release-Note: >Audit-Trail: From: "David G. Andersen" To: freebsd-gnats-submit@FreeBSD.org, dga@lcs.mit.edu Cc: Subject: Re: bin/67467: df -m and -g incorrect with negative avail Date: Tue, 1 Jun 2004 22:08:14 -0400 The problem is with the fsbtoblk macro on line 401 of /usr/src/bin/df.c #define fsbtoblk(num, fsbs, bs) \ (((fsbs) != 0 && (fsbs) < (bs)) ? \ (num) / ((bs) / (fsbs)) : (num) * ((fsbs) / (bs))) The problem only occurs when the user specifies a blocksize larger than the filesystem blocksize (probably 4k), invoking this part of the code: (num) / ((bs) / (fsbs)) Unfortunately, the result of this is of the type of fsbs (which is a u_int64_t), instead of being of the type of num (int64_t). The easiest solution is an explicit cast in the macro: (num) / (intmax_t)((bs) / (fsbs)) : (num) * ((fsbs) / (bs))) This could result in a premature cast out of unsignedness iff fsbs = 1 and bs > 2^63 and num is an unsigned quantity, but it's safe in other situations. If gcc extensions are kosher, then it would better be written as: (num) / (typeof(num))((bs) / (fsbs)) : (num) * ((fsbs) / (bs))) since typeof seems to be used elsewhere in the kernel, I assume this is acceptable. From: "David G. Andersen" To: freebsd-gnats-submit@FreeBSD.org, dga@lcs.mit.edu Cc: Subject: Re: bin/67467: df -m and -g incorrect with negative avail Date: Wed, 2 Jun 2004 11:07:36 -0400 Whoops. That should be: (num) / (typeof(num))((bs) / (fsbs)) : (num) * (typeof(num))((fsbs) / (bs))) to keep the types the same across both sides of the conditional. Sorry. State-Changed-From-To: open->closed State-Changed-By: das State-Changed-When: Fri Jun 4 02:30:57 PDT 2004 State-Changed-Why: Thanks for the analysis. It turns out that this problem was fixed last month in src/bin/df/df.c,v 1.56 using your first proposal. However, in reviewing the code, I found that I was able to save 775 bytes and a small piece of my sanity by making fsbtoblk() a true function. This should address your concern about the ambiguity in the type of num. (As for gcc extensions, we use them in some places, but we try not to depend on them when possible.) http://www.freebsd.org/cgi/query-pr.cgi?pr=67467 >Unformatted: