2007-10-08 21:55:58

by Dmitri Vorobiev

[permalink] [raw]
Subject: [PATCH] NTFS error messages: replace static char pointers by static char arrays

Hi,

The patch below contains a small code clean-up for the NTFS driver: all
static char pointers to error message strings have been replaced by
static char arrays.

Please apply if you like it.

Signed-off-by: Dmitri Vorobiev <[email protected]>
---
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 1c08fef..b883537 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -869,7 +869,7 @@ static int ntfs_external_attr_find(const ATTR_TYPE type,
ntfschar *al_name;
u32 al_name_len;
int err = 0;
- static const char *es = " Unmount and run chkdsk.";
+ static const char es[] = " Unmount and run chkdsk.";

ni = ctx->ntfs_ino;
base_ni = ctx->base_ntfs_ino;
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index b532a73..82ac179 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -1870,7 +1870,7 @@ int ntfs_read_inode_mount(struct inode *vi)
} else /* if (!err) */ {
ATTR_LIST_ENTRY *al_entry, *next_al_entry;
u8 *al_end;
- static const char *es = " Not allowed. $MFT is corrupt. "
+ static const char es[] = " Not allowed. $MFT is corrupt. "
"You should run chkdsk.";

ntfs_debug("Attribute list attribute found in $MFT.");
@@ -2332,7 +2332,7 @@ int ntfs_show_options(struct seq_file *sf, struct
vfsmount *mnt)

#ifdef NTFS_RW

-static const char *es = " Leaving inconsistent metadata. Unmount and
run "
+static const char es[] = " Leaving inconsistent metadata. Unmount and
run "
"chkdsk.";

/**
@@ -2368,7 +2368,7 @@ int ntfs_truncate(struct inode *vi)
ntfs_attr_search_ctx *ctx;
MFT_RECORD *m;
ATTR_RECORD *a;
- const char *te = " Leaving file length out of sync with i_size.";
+ const char te[] = " Leaving file length out of sync with i_size.";
int err, mp_size, size_change, alloc_change;
u32 attr_len;

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 2ad5c8b..a560bb0 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -409,7 +409,7 @@ void __mark_mft_record_dirty(ntfs_inode *ni)
__mark_inode_dirty(VFS_I(base_ni), I_DIRTY_SYNC | I_DIRTY_DATASYNC);
}

-static const char *ntfs_please_email = "Please email "
+static const char ntfs_please_email[] = "Please email "
"[email protected] and say that you saw "
"this message. Thank you.";

@@ -1106,7 +1106,7 @@ bool ntfs_may_write_mft_record(ntfs_volume *vol,
const unsigned long mft_no,
return true;
}

-static const char *es = " Leaving inconsistent metadata. Unmount and
run "
+static const char es[] = " Leaving inconsistent metadata. Unmount and
run "
"chkdsk.";

/**
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 90c4e3a..a03ca16 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -460,7 +460,7 @@ static int ntfs_remount(struct super_block *sb, int
*flags, char *opt)
* have occured.
*/
if ((sb->s_flags & MS_RDONLY) && !(*flags & MS_RDONLY)) {
- static const char *es = ". Cannot remount read-write.";
+ static const char es[] = ". Cannot remount read-write.";

/* Remounting read-write. */
if (NVolErrors(vol)) {
@@ -647,7 +647,7 @@ not_ntfs:
static struct buffer_head *read_ntfs_boot_sector(struct super_block *sb,
const int silent)
{
- const char *read_err_str = "Unable to read %s boot sector.";
+ const char read_err_str[] = "Unable to read %s boot sector.";
struct buffer_head *bh_primary, *bh_backup;
sector_t nr_blocks = NTFS_SB(sb)->nr_blocks;

@@ -1756,9 +1756,9 @@ static bool load_system_files(ntfs_volume *vol)
#ifdef NTFS_RW
/* Get mft mirror inode compare the contents of $MFT and $MFTMirr. */
if (!load_and_init_mft_mirror(vol) || !check_mft_mirror(vol)) {
- static const char *es1 = "Failed to load $MFTMirr";
- static const char *es2 = "$MFTMirr does not match $MFT";
- static const char *es3 = ". Run ntfsfix and/or chkdsk.";
+ static const char es1[] = "Failed to load $MFTMirr";
+ static const char es2[] = "$MFTMirr does not match $MFT";
+ static const char es3[] = ". Run ntfsfix and/or chkdsk.";

/* If a read-write mount, convert it to a read-only mount. */
if (!(sb->s_flags & MS_RDONLY)) {
@@ -1880,11 +1880,12 @@ get_ctx_vol_failed:
#ifdef NTFS_RW
/* Make sure that no unsupported volume flags are set. */
if (vol->vol_flags & VOLUME_MUST_MOUNT_RO_MASK) {
- static const char *es1a = "Volume is dirty";
- static const char *es1b = "Volume has been modified by chkdsk";
- static const char *es1c = "Volume has unsupported flags set";
- static const char *es2a = ". Run chkdsk and mount in Windows.";
- static const char *es2b = ". Mount in Windows.";
+ static const char es1a[] = "Volume is dirty";
+ static const char es1b[] = "Volume has been modified by chkdsk";
+ static const char es1c[] = "Volume has unsupported flags set";
+ static const char es2a[] = ". Run chkdsk and mount in "
+ "Windows.";
+ static const char es2b[] = ". Mount in Windows.";
const char *es1, *es2;

es2 = es2a;
@@ -1926,9 +1927,9 @@ get_ctx_vol_failed:
rp = NULL;
if (!load_and_check_logfile(vol, &rp) ||
!ntfs_is_logfile_clean(vol->logfile_ino, rp)) {
- static const char *es1a = "Failed to load $LogFile";
- static const char *es1b = "$LogFile is not clean";
- static const char *es2 = ". Mount in Windows.";
+ static const char es1a[] = "Failed to load $LogFile";
+ static const char es1b[] = "$LogFile is not clean";
+ static const char es2[] = ". Mount in Windows.";
const char *es1;

es1 = !vol->logfile_ino ? es1a : es1b;
@@ -1974,10 +1975,10 @@ get_ctx_vol_failed:
*/
err = check_windows_hibernation_status(vol);
if (unlikely(err)) {
- static const char *es1a = "Failed to determine if Windows is "
+ static const char es1a[] = "Failed to determine if Windows is "
"hibernated";
- static const char *es1b = "Windows is hibernated";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1b[] = "Windows is hibernated";
+ static const char es2[] = ". Run chkdsk.";
const char *es1;

es1 = err < 0 ? es1a : es1b;
@@ -2002,9 +2003,9 @@ get_ctx_vol_failed:
/* If (still) a read-write mount, mark the volume dirty. */
if (!(sb->s_flags & MS_RDONLY) &&
ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY)) {
- static const char *es1 = "Failed to set dirty bit in volume "
+ static const char es1[] = "Failed to set dirty bit in volume "
"information flags";
- static const char *es2 = ". Run chkdsk.";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2030,8 +2031,9 @@ get_ctx_vol_failed:
*/
if (!(sb->s_flags & MS_RDONLY) && (vol->major_ver > 1) &&
ntfs_set_volume_flags(vol, VOLUME_MOUNTED_ON_NT4)) {
- static const char *es1 = "Failed to set NT4 compatibility flag";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to set NT4 compatibility "
+ "flag";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2049,8 +2051,8 @@ get_ctx_vol_failed:
/* If (still) a read-write mount, empty the logfile. */
if (!(sb->s_flags & MS_RDONLY) &&
!ntfs_empty_logfile(vol->logfile_ino)) {
- static const char *es1 = "Failed to empty $LogFile";
- static const char *es2 = ". Mount in Windows.";
+ static const char es1[] = "Failed to empty $LogFile";
+ static const char es2[] = ". Mount in Windows.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2089,8 +2091,8 @@ get_ctx_vol_failed:
#ifdef NTFS_RW
/* Find the quota file, load it if present, and set it up. */
if (!load_and_init_quota(vol)) {
- static const char *es1 = "Failed to load $Quota";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to load $Quota";
+ static const char es2[] = ". Run chkdsk.";

/* If a read-write mount, convert it to a read-only mount. */
if (!(sb->s_flags & MS_RDONLY)) {
@@ -2113,8 +2115,8 @@ get_ctx_vol_failed:
/* If (still) a read-write mount, mark the quotas out of date. */
if (!(sb->s_flags & MS_RDONLY) &&
!ntfs_mark_quotas_out_of_date(vol)) {
- static const char *es1 = "Failed to mark quotas out of date";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to mark quotas out of date";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2133,8 +2135,8 @@ get_ctx_vol_failed:
* it, and set it up.
*/
if (!load_and_init_usnjrnl(vol)) {
- static const char *es1 = "Failed to load $UsnJrnl";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to load $UsnJrnl";
+ static const char es2[] = ". Run chkdsk.";

/* If a read-write mount, convert it to a read-only mount. */
if (!(sb->s_flags & MS_RDONLY)) {
@@ -2156,9 +2158,9 @@ get_ctx_vol_failed:
}
/* If (still) a read-write mount, stamp the transaction log. */
if (!(sb->s_flags & MS_RDONLY) && !ntfs_stamp_usnjrnl(vol)) {
- static const char *es1 = "Failed to stamp transaction log "
+ static const char es1[] = "Failed to stamp transaction log "
"($UsnJrnl)";
- static const char *es2 = ". Run chkdsk.";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2389,13 +2391,13 @@ static void ntfs_put_super(struct super_block *sb)
mutex_unlock(&vol->mft_ino->i_mutex);
write_inode_now(vol->mft_ino, 1);
if (!list_empty(&sb->s_dirty)) {
- static const char *_s1 = "inodes";
- static const char *_s2 = "";
+ static const char _s1[] = "inodes";
+ static const char _s2[] = "";
s1 = _s1;
s2 = _s2;
} else {
- static const char *_s1 = "mft pages";
- static const char *_s2 = "They have been thrown "
+ static const char _s1[] = "mft pages";
+ static const char _s2[] = "They have been thrown "
"away. ";
s1 = _s1;
s2 = _s2;


2007-10-09 12:40:54

by Ahmed S. Darwish

[permalink] [raw]
Subject: Re: [PATCH] NTFS error messages: replace static char pointers by static char arrays

On Tue, Oct 09, 2007 at 01:55:42AM +0400, Dmitri Vorobiev wrote:
> Hi,
>
> The patch below contains a small code clean-up for the NTFS driver: all
> static char pointers to error message strings have been replaced by
> static char arrays.
>

Hi Dmitri,

Isn't the only difference between char *c = "msg" and char c[] = "msg" is
that the first is a non-const pointer to a const char array while the second
is a modifiable char array ?

If so, what's the point of the patch ?

Thanks,

--
Ahmed S. Darwish
HomePage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com

2007-10-09 18:46:20

by Dmitri Vorobiev

[permalink] [raw]
Subject: Re: [PATCH] NTFS error messages: replace static char pointers by static char arrays

Ahmed S. Darwish wrote:
> On Tue, Oct 09, 2007 at 01:55:42AM +0400, Dmitri Vorobiev wrote:
>> Hi,
>>
>> The patch below contains a small code clean-up for the NTFS driver: all
>> static char pointers to error message strings have been replaced by
>> static char arrays.
>>
>
> Hi Dmitri,

Hi,

First off, I hate to admit that my e-mail application broke the patch I sent in the first message in this thread. I am reposting the patch now in the hope that it'll get through intact.

>
> Isn't the only difference between char *c = "msg" and char c[] = "msg" is
> that the first is a non-const pointer to a const char array while the second
> is a modifiable char array ?

The second case creates only one variable while the first one creates an extra pointer variable. Detailed explanation is included below, and you may also want to refer to the following LKML thread: http://lkml.org/lkml/2005/6/21/16

>
> If so, what's the point of the patch ?

The point was to (hopefully) improve the NTFS driver, of course :)

I am using below my reply to a private email I received concerning this patch.

Briefly put, this modification removes the extra pointer variable from the data section of the resulting object file.

Consider two source files:

<<<

dmvo@cipher:/tmp$ cat c.c
static const char *c = "hello";

const char *f()
{
return c;
}
dmvo@cipher:/tmp$ cat d.c
static const char c[] = "hello";

const char *f()
{
return c;
}
dmvo@cipher:/tmp$

>>>

These files demonstrate the essence of the modification that I proposed for the error messages in the Linux NTFS driver. Now let's try to compile these two source files into the Assembly code (I am using i386 Assembly here, but this shouldn't matter since we're talking about C language and ELF files - both are standardized). The f() function has deliberately been made global to convince the compiler that this function should not be optimized away.

This is how the first file looks when compiled:

<<<

dmvo@cipher:/tmp$ gcc -S -O2 -o- c.c
.file "c.c"
.section .rodata.str1.1,"aMS",@progbits,1
.LC0:
.string "hello"
.data
.align 4
.type c, @object
.size c, 4
c:
.long .LC0
.text
.globl f
.type f, @function
f:
pushl %ebp
movl %esp, %ebp
movl c, %eax
popl %ebp
ret
.size f, .-f
.ident "GCC: (GNU) 4.0.3 (Ubuntu 4.0.3-1ubuntu5)"
.section .note.GNU-stack,"",@progbits
dmvo@cipher:/tmp$

>>>

What we're having here are essentially the following things:

1) the .text section containing the code of the function f();
2) the .rodata section where the compiler put the six bytes of the C string;
3) the variable c in the .data section. This variable gets initialized to the address of the "hello" string (note the LC0 label).

Now let's turn to the second case:

<<<

dmvo@cipher:/tmp$ gcc -S -O2 -o- d.c
.file "d.c"
.section .rodata
.type c, @object
.size c, 6
c:
.string "hello"
.text
.globl f
.type f, @function
f:
pushl %ebp
movl %esp, %ebp
movl $c, %eax
popl %ebp
ret
.size f, .-f
.ident "GCC: (GNU) 4.0.3 (Ubuntu 4.0.3-1ubuntu5)"
.section .note.GNU-stack,"",@progbits
dmvo@cipher:/tmp$

>>>

The difference between this case and the previous one is that the .data section does not exist any more. Instead, the C string ceased to be an anonymous object, and is now referred by the symbol c, the latter residing in the .rodata section. Thus, the computational resources needed by the code have been reduced a little bit.

Another way to see that the second version does reduce space is to further assemble the sources into the ELF object files and check the size of resulting ELF objects and the sizes of the sections in these object files:

<<<

dmvo@cipher:/tmp$ size c.o
text data bss dec hex filename
16 4 0 20 14 c.o
dmvo@cipher:/tmp$ cc -c -O2 d.c
dmvo@cipher:/tmp$ size d.o
text data bss dec hex filename
16 0 0 16 10 d.o
dmvo@cipher:/tmp$ ls -la
-rw-r--r-- 1 dmvo users 876 2007-10-09 20:20 c.o
-rw-r--r-- 1 dmvo users 816 2007-10-09 20:20 d.o
dmvo@cipher:/tmp$

>>>

Thanks,

Dmitri

Signed-off-by Dmitri Vorobiev <[email protected]>

--
diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 1c08fef..b883537 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -869,7 +869,7 @@ static int ntfs_external_attr_find(const ATTR_TYPE type,
ntfschar *al_name;
u32 al_name_len;
int err = 0;
- static const char *es = " Unmount and run chkdsk.";
+ static const char es[] = " Unmount and run chkdsk.";

ni = ctx->ntfs_ino;
base_ni = ctx->base_ntfs_ino;
diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index b532a73..82ac179 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -1870,7 +1870,7 @@ int ntfs_read_inode_mount(struct inode *vi)
} else /* if (!err) */ {
ATTR_LIST_ENTRY *al_entry, *next_al_entry;
u8 *al_end;
- static const char *es = " Not allowed. $MFT is corrupt. "
+ static const char es[] = " Not allowed. $MFT is corrupt. "
"You should run chkdsk.";

ntfs_debug("Attribute list attribute found in $MFT.");
@@ -2332,7 +2332,7 @@ int ntfs_show_options(struct seq_file *sf, struct vfsmount *mnt)

#ifdef NTFS_RW

-static const char *es = " Leaving inconsistent metadata. Unmount and run "
+static const char es[] = " Leaving inconsistent metadata. Unmount and run "
"chkdsk.";

/**
@@ -2368,7 +2368,7 @@ int ntfs_truncate(struct inode *vi)
ntfs_attr_search_ctx *ctx;
MFT_RECORD *m;
ATTR_RECORD *a;
- const char *te = " Leaving file length out of sync with i_size.";
+ const char te[] = " Leaving file length out of sync with i_size.";
int err, mp_size, size_change, alloc_change;
u32 attr_len;

diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 2ad5c8b..a560bb0 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -409,7 +409,7 @@ void __mark_mft_record_dirty(ntfs_inode *ni)
__mark_inode_dirty(VFS_I(base_ni), I_DIRTY_SYNC | I_DIRTY_DATASYNC);
}

-static const char *ntfs_please_email = "Please email "
+static const char ntfs_please_email[] = "Please email "
"[email protected] and say that you saw "
"this message. Thank you.";

@@ -1106,7 +1106,7 @@ bool ntfs_may_write_mft_record(ntfs_volume *vol, const unsigned long mft_no,
return true;
}

-static const char *es = " Leaving inconsistent metadata. Unmount and run "
+static const char es[] = " Leaving inconsistent metadata. Unmount and run "
"chkdsk.";

/**
diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
index 90c4e3a..a03ca16 100644
--- a/fs/ntfs/super.c
+++ b/fs/ntfs/super.c
@@ -460,7 +460,7 @@ static int ntfs_remount(struct super_block *sb, int *flags, char *opt)
* have occured.
*/
if ((sb->s_flags & MS_RDONLY) && !(*flags & MS_RDONLY)) {
- static const char *es = ". Cannot remount read-write.";
+ static const char es[] = ". Cannot remount read-write.";

/* Remounting read-write. */
if (NVolErrors(vol)) {
@@ -647,7 +647,7 @@ not_ntfs:
static struct buffer_head *read_ntfs_boot_sector(struct super_block *sb,
const int silent)
{
- const char *read_err_str = "Unable to read %s boot sector.";
+ const char read_err_str[] = "Unable to read %s boot sector.";
struct buffer_head *bh_primary, *bh_backup;
sector_t nr_blocks = NTFS_SB(sb)->nr_blocks;

@@ -1756,9 +1756,9 @@ static bool load_system_files(ntfs_volume *vol)
#ifdef NTFS_RW
/* Get mft mirror inode compare the contents of $MFT and $MFTMirr. */
if (!load_and_init_mft_mirror(vol) || !check_mft_mirror(vol)) {
- static const char *es1 = "Failed to load $MFTMirr";
- static const char *es2 = "$MFTMirr does not match $MFT";
- static const char *es3 = ". Run ntfsfix and/or chkdsk.";
+ static const char es1[] = "Failed to load $MFTMirr";
+ static const char es2[] = "$MFTMirr does not match $MFT";
+ static const char es3[] = ". Run ntfsfix and/or chkdsk.";

/* If a read-write mount, convert it to a read-only mount. */
if (!(sb->s_flags & MS_RDONLY)) {
@@ -1880,11 +1880,12 @@ get_ctx_vol_failed:
#ifdef NTFS_RW
/* Make sure that no unsupported volume flags are set. */
if (vol->vol_flags & VOLUME_MUST_MOUNT_RO_MASK) {
- static const char *es1a = "Volume is dirty";
- static const char *es1b = "Volume has been modified by chkdsk";
- static const char *es1c = "Volume has unsupported flags set";
- static const char *es2a = ". Run chkdsk and mount in Windows.";
- static const char *es2b = ". Mount in Windows.";
+ static const char es1a[] = "Volume is dirty";
+ static const char es1b[] = "Volume has been modified by chkdsk";
+ static const char es1c[] = "Volume has unsupported flags set";
+ static const char es2a[] = ". Run chkdsk and mount in "
+ "Windows.";
+ static const char es2b[] = ". Mount in Windows.";
const char *es1, *es2;

es2 = es2a;
@@ -1926,9 +1927,9 @@ get_ctx_vol_failed:
rp = NULL;
if (!load_and_check_logfile(vol, &rp) ||
!ntfs_is_logfile_clean(vol->logfile_ino, rp)) {
- static const char *es1a = "Failed to load $LogFile";
- static const char *es1b = "$LogFile is not clean";
- static const char *es2 = ". Mount in Windows.";
+ static const char es1a[] = "Failed to load $LogFile";
+ static const char es1b[] = "$LogFile is not clean";
+ static const char es2[] = ". Mount in Windows.";
const char *es1;

es1 = !vol->logfile_ino ? es1a : es1b;
@@ -1974,10 +1975,10 @@ get_ctx_vol_failed:
*/
err = check_windows_hibernation_status(vol);
if (unlikely(err)) {
- static const char *es1a = "Failed to determine if Windows is "
+ static const char es1a[] = "Failed to determine if Windows is "
"hibernated";
- static const char *es1b = "Windows is hibernated";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1b[] = "Windows is hibernated";
+ static const char es2[] = ". Run chkdsk.";
const char *es1;

es1 = err < 0 ? es1a : es1b;
@@ -2002,9 +2003,9 @@ get_ctx_vol_failed:
/* If (still) a read-write mount, mark the volume dirty. */
if (!(sb->s_flags & MS_RDONLY) &&
ntfs_set_volume_flags(vol, VOLUME_IS_DIRTY)) {
- static const char *es1 = "Failed to set dirty bit in volume "
+ static const char es1[] = "Failed to set dirty bit in volume "
"information flags";
- static const char *es2 = ". Run chkdsk.";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2030,8 +2031,9 @@ get_ctx_vol_failed:
*/
if (!(sb->s_flags & MS_RDONLY) && (vol->major_ver > 1) &&
ntfs_set_volume_flags(vol, VOLUME_MOUNTED_ON_NT4)) {
- static const char *es1 = "Failed to set NT4 compatibility flag";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to set NT4 compatibility "
+ "flag";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2049,8 +2051,8 @@ get_ctx_vol_failed:
/* If (still) a read-write mount, empty the logfile. */
if (!(sb->s_flags & MS_RDONLY) &&
!ntfs_empty_logfile(vol->logfile_ino)) {
- static const char *es1 = "Failed to empty $LogFile";
- static const char *es2 = ". Mount in Windows.";
+ static const char es1[] = "Failed to empty $LogFile";
+ static const char es2[] = ". Mount in Windows.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2089,8 +2091,8 @@ get_ctx_vol_failed:
#ifdef NTFS_RW
/* Find the quota file, load it if present, and set it up. */
if (!load_and_init_quota(vol)) {
- static const char *es1 = "Failed to load $Quota";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to load $Quota";
+ static const char es2[] = ". Run chkdsk.";

/* If a read-write mount, convert it to a read-only mount. */
if (!(sb->s_flags & MS_RDONLY)) {
@@ -2113,8 +2115,8 @@ get_ctx_vol_failed:
/* If (still) a read-write mount, mark the quotas out of date. */
if (!(sb->s_flags & MS_RDONLY) &&
!ntfs_mark_quotas_out_of_date(vol)) {
- static const char *es1 = "Failed to mark quotas out of date";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to mark quotas out of date";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2133,8 +2135,8 @@ get_ctx_vol_failed:
* it, and set it up.
*/
if (!load_and_init_usnjrnl(vol)) {
- static const char *es1 = "Failed to load $UsnJrnl";
- static const char *es2 = ". Run chkdsk.";
+ static const char es1[] = "Failed to load $UsnJrnl";
+ static const char es2[] = ". Run chkdsk.";

/* If a read-write mount, convert it to a read-only mount. */
if (!(sb->s_flags & MS_RDONLY)) {
@@ -2156,9 +2158,9 @@ get_ctx_vol_failed:
}
/* If (still) a read-write mount, stamp the transaction log. */
if (!(sb->s_flags & MS_RDONLY) && !ntfs_stamp_usnjrnl(vol)) {
- static const char *es1 = "Failed to stamp transaction log "
+ static const char es1[] = "Failed to stamp transaction log "
"($UsnJrnl)";
- static const char *es2 = ". Run chkdsk.";
+ static const char es2[] = ". Run chkdsk.";

/* Convert to a read-only mount. */
if (!(vol->on_errors & (ON_ERRORS_REMOUNT_RO |
@@ -2389,13 +2391,13 @@ static void ntfs_put_super(struct super_block *sb)
mutex_unlock(&vol->mft_ino->i_mutex);
write_inode_now(vol->mft_ino, 1);
if (!list_empty(&sb->s_dirty)) {
- static const char *_s1 = "inodes";
- static const char *_s2 = "";
+ static const char _s1[] = "inodes";
+ static const char _s2[] = "";
s1 = _s1;
s2 = _s2;
} else {
- static const char *_s1 = "mft pages";
- static const char *_s2 = "They have been thrown "
+ static const char _s1[] = "mft pages";
+ static const char _s2[] = "They have been thrown "
"away. ";
s1 = _s1;
s2 = _s2;

2007-10-09 18:55:21

by Philipp Matthias Hahn

[permalink] [raw]
Subject: Re: [PATCH] NTFS error messages: replace static char pointers by static char arrays

Hello!

On Tue, Oct 09, 2007 at 02:40:35PM +0200, Ahmed S. Darwish wrote:
> On Tue, Oct 09, 2007 at 01:55:42AM +0400, Dmitri Vorobiev wrote:
> > The patch below contains a small code clean-up for the NTFS driver: all
> > static char pointers to error message strings have been replaced by
> > static char arrays.

char * a = "a"; // pointer and content can be changed
const char * b = "b"; // the thing pointed to is const
char * const c = "c"; // the pointer is const
const char * const d = "d"; // pointer and content can't be changed

void foo(void) {
*a = 'A';
a++;
*b = 'B'; // error: assignment of read-only location
b++;
*c = 'C';
c++; // error: increment of read-only variable 'c'
*d = 'D'; // error: assignment of read-only location
d++; // error: increment of read-only variable 'd'
}

> Isn't the only difference between char *c = "msg" and char c[] = "msg" is
> that the first is a non-const pointer to a const char array while the second
> is a modifiable char array ?

$ cat [ab].c
const char *a = "a";
const char b[] = "b";
$ gcc -c [ab].c
$ size [ab].o
text data bss dec hex filename
2 4 0 6 6 a.o
2 0 0 2 2 b.o

'a' has two entries: one for the named read-writeable pointer, and one for the
anonymous read-only string, the pointer points to.
'b' has a single entry: just the named read-only string.

BYtE
Philipp
--
/ / (_)__ __ ____ __ Philipp Hahn
/ /__/ / _ \/ // /\ \/ /
/____/_/_//_/\_,_/ /_/\_\ [email protected]

2007-10-09 22:03:57

by Ahmed S. Darwish

[permalink] [raw]
Subject: Re: [PATCH] NTFS error messages: replace static char pointers by static char arrays

On Tue, Oct 09, 2007 at 08:33:59PM +0200, Philipp Matthias Hahn wrote:
> Hello!
>
> On Tue, Oct 09, 2007 at 02:40:35PM +0200, Ahmed S. Darwish wrote:
> > On Tue, Oct 09, 2007 at 01:55:42AM +0400, Dmitri Vorobiev wrote:
> > > The patch below contains a small code clean-up for the NTFS driver: all
> > > static char pointers to error message strings have been replaced by
> > > static char arrays.
>
> char * a = "a"; // pointer and content can be changed

Only the pointer can be changed here. AFAIK "a" is a const string.

> const char * b = "b"; // the thing pointed to is const

The "const" here is redundant (just useful for forcing the compiler to
prevent us from shooting our feet). The "b" string is already constant.

> char * const c = "c"; // the pointer is const
> const char * const d = "d"; // pointer and content can't be changed
>
> void foo(void) {
> *a = 'A';

This will segfault.

> a++;
> *b = 'B'; // error: assignment of read-only location
> b++;
> *c = 'C';

Last line will segfault too.

> c++; // error: increment of read-only variable 'c'
> *d = 'D'; // error: assignment of read-only location
> d++; // error: increment of read-only variable 'd'
> }
>

Please continue below.

> > Isn't the only difference between char *c = "msg" and char c[] = "msg" is
> > that the first is a non-const pointer to a const char array while the second
> > is a modifiable char array ?
>
> $ cat [ab].c
> const char *a = "a";
> const char b[] = "b";
> $ gcc -c [ab].c
> $ size [ab].o
> text data bss dec hex filename
> 2 4 0 6 6 a.o
> 2 0 0 2 2 b.o
>
> 'a' has two entries: one for the named read-writeable pointer, and one for the
> anonymous read-only string, the pointer points to.
> 'b' has a single entry: just the named read-only string.
>

Got the point, Thanks!.

--
Ahmed S. Darwish
HomePage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com

2007-10-13 22:13:53

by folkert

[permalink] [raw]
Subject: Re: [PATCH] NTFS error messages: replace static char pointers by static char arrays

>>> The patch below contains a small code clean-up for the NTFS driver: all
>>> static char pointers to error message strings have been replaced by
>>> static char arrays.

While doing that clean-up, shouldn't these be converted as well?

folkert@muur:/usr/src/linux$ find . -name \*.c -print0 | xargs -0 grep "^ *char *\* *[a-zA-Z0-9_]* *= *\".*\".*;"
./arch/mips/tx4927/toshiba_rbtx4927/toshiba_rbtx4927_setup.c:char *toshiba_name = "";
./arch/ppc/boot/simple/misc-embedded.c:char *bootrom_cmdline = "";
./arch/blackfin/mach-bf561/boards/ezkit.c:char *bfin_board_name = "ADDS-BF561-EZKIT";
./arch/blackfin/mach-bf561/boards/generic_board.c:char *bfin_board_name = "UNKNOWN BOARD";
./arch/blackfin/mach-bf561/boards/tepla.c:char *bfin_board_name = "Tepla-BF561";
./arch/blackfin/mach-bf561/boards/cm_bf561.c:char *bfin_board_name = "Bluetechnix CM BF561";
./arch/blackfin/mach-bf533/boards/ezkit.c:char *bfin_board_name = "ADDS-BF533-EZKIT";
./arch/blackfin/mach-bf533/boards/cm_bf533.c:char *bfin_board_name = "Bluetechnix CM BF533";
./arch/blackfin/mach-bf533/boards/stamp.c:char *bfin_board_name = "ADDS-BF533-STAMP";
./arch/blackfin/mach-bf533/boards/generic_board.c:char *bfin_board_name = "UNKNOWN BOARD";
./arch/blackfin/mach-bf537/boards/stamp.c:char *bfin_board_name = "ADDS-BF537-STAMP";
./arch/blackfin/mach-bf537/boards/generic_board.c:char *bfin_board_name = "UNKNOWN BOARD";
./arch/blackfin/mach-bf537/boards/pnav10.c:char *bfin_board_name = "PNAV-1.0";
./arch/blackfin/mach-bf537/boards/cm_bf537.c:char *bfin_board_name = "Bluetechnix CM BF537";
./arch/blackfin/mach-bf548/boards/ezkit.c:char *bfin_board_name = "ADSP-BF548-EZKIT";
./drivers/net/pcmcia/fmvj18x_cs.c: char *card_name = "unknown";
./drivers/atm/fore200e_mkfirm.c:char* default_basename = "pca200e"; /* was initially written for the PCA-200E firmware */
./drivers/atm/fore200e_mkfirm.c:char* default_infname = "<stdin>";
./drivers/atm/fore200e_mkfirm.c:char* default_outfname = "<stdout>";
./drivers/scsi/aic7xxx_old.c: char *channel = "";
./drivers/scsi/aic7xxx_old/aic7xxx_proc.c: char *channel = "";
./drivers/scsi/aic7xxx_old/aic7xxx_proc.c: char *ultra = "";
./drivers/scsi/aic7xxx_old/aic7xxx_proc.c: char *wide = "Narrow ";
./drivers/isdn/i4l/isdn_ppp.c:char *isdn_ppp_revision = "$Revision: 1.1.2.3 $";
./drivers/isdn/i4l/isdn_tty.c:char *isdn_tty_revision = "$Revision: 1.1.2.3 $";
./drivers/isdn/i4l/isdn_net.c:char *isdn_net_revision = "$Revision: 1.1.2.2 $";
./drivers/isdn/i4l/isdn_audio.c:char *isdn_audio_revision = "$Revision: 1.1.2.2 $";
./drivers/isdn/i4l/isdn_v110.c:char *isdn_v110_revision = "$Revision: 1.1.2.2 $";
./drivers/isdn/hysdn/hysdn_net.c:char *hysdn_net_revision = "$Revision: 1.8.6.4 $";
./drivers/isdn/hardware/eicon/diva_didd.c:char *DRIVERRELEASE_DIDD = "2.0";
./drivers/isdn/hardware/eicon/divasmain.c:char *DRIVERRELEASE_DIVAS = "2.0";
./drivers/isdn/hardware/eicon/divasi.c:char *DRIVERRELEASE_IDI = "2.0";
./drivers/isdn/hardware/eicon/divamnt.c:char *DRIVERRELEASE_MNT = "2.0";
./drivers/serial/mcfserial.c:char *mcfrs_drivername = "ColdFire internal UART serial driver version 1.00\n";


Folkert van Heusden

--
MultiTail is a versatile tool for watching logfiles and output of
commands. Filtering, coloring, merging, diff-view, etc.
http://www.vanheusden.com/multitail/
----------------------------------------------------------------------
Phone: +31-6-41278122, PGP-key: 1F28D8AE, http://www.vanheusden.com