Re: [PATCH v2 21/21] t1308: add a test case on open a config directory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Duy,

On Wed, 3 May 2017, Nguyễn Thái Ngọc Duy wrote:

> We don't support config-as-a-directory (maybe someday we will?). Make
> sure we consistently fail in this case, which should happen on platforms
> where fopen(<directory>) returns non-NULL if FREAD_READS_DIRECTORIES is
> defined.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>

How about saying that we make sure that this is the case by adding a test
case? Otherwise the reader might (and this reader certainly did) expect a
code change in a .c file.

I reviewed this patch series, and am happy with the overall diff.

My main problem: it should be organized in a more intuitively-graspable
way, i.e. put all the fopen() -> fopen_or_warn() changes into a single
patch, separate out changes (drive-by test fixing for Windows and, ahem,
whitespace changes) into their own commits. Then those commits should be
ordered to relate an easy-flowing story.

Other than that: well done,
Dscho

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]