linux-ext4 - Re: [PATCH 1/3] mke2fs: print a message when creating a regular file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5367A445.9070209@redhat.com>
Date:	Mon, 05 May 2014 09:46:29 -0500
From:	Eric Sandeen <sandeen@...hat.com>
To:	"Theodore Ts'o" <tytso@....edu>,
	Lukáš Czerner <lczerner@...hat.com>
CC:	Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 1/3] mke2fs: print a message when creating a regular file

On 5/5/14, 8:52 AM, Theodore Ts'o wrote:
> On Mon, May 05, 2014 at 03:41:01PM +0200, Lukáš Czerner wrote:
>> On Mon, 5 May 2014, Theodore Ts'o wrote:
>>
>>> Date: Mon,  5 May 2014 09:04:02 -0400
>>> From: Theodore Ts'o <tytso@....edu>
>>> To: Ext4 Developers List <linux-ext4@...r.kernel.org>
>>> Cc: Theodore Ts'o <tytso@....edu>
>>> Subject: [PATCH 1/3] mke2fs: print a message when creating a regular file
>>
>> Description is missing.
> 
> I didn't think more of a description was needed; can you suggest one?

Well, sometimes the summary is enough - if this patch just added a printf,
the summary covers the functional change.  However, I think it's useful
to see the rationale for a change, not just the description of the change.

"Close file descriptors before exit" doesn't really need any rationale,
but changing program behavior and/or output might.

AFAIK, nobody ever complained that a changelog was too descriptive or
informative.  ;)

>>>  	/* Can't undo discard ... */
>>> -	if (!noaction && discard && (io_ptr != undo_io_manager)) {
>>> +	if (!noaction && discard && dev_size && (io_ptr != undo_io_manager)) {
>>
>> I wonder whether it's possible not to have dev_size set at this point ?
> 
> I checked, and I don't believe so.  ext2fs_get_device_size2() never
> returns EXT2_ET_UNIMPLEMENTED any more; and it hasn't for quite some
> time, since it will use st_size for a regular file or do a binary
> search trying to figure out the device size in the worst case.  So in
> fact, there are some code paths we can eliminate in misc/mke2fs.c
> which will simplify this analysis.
> 
> Even if there is some case in the future where dev_size could be left
> unset, it will be initialized to zero, at which point we will fail
> safe by skipping the mke2fs_discard_device() call.

I kind of lost the thread here; if it is impossible to be unset, why
add the check?

And if (!dev_size) what would happen on this path?  I guess I need to get
all the pending patches applied to see what's changed in this area.

-Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html