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 <dmitri.vorobiev@gmail.com>
---
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)
  ...
[ message continues ]

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
-

[ message continues ]

Hi,



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;
}

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

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
        ...
[ message continues ]

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 = ...
[ message continues ]

Hello!


      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'

$ 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
 / /__/ / _ \/ // /\ \/ /
/____/_/_//_/\_,_/ /_/\_\ pmhahn@titan.lahn.de
-

[ message continues ]

The "const" here is redundant (just useful for forcing the compiler to




Got the point, Thanks!.

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

[ message continues ]