Hi,
NAK.
As Al Viro said in reply to one of your other patches, if you look at the
assembler output you will see that the 'char te[] = "string"' generates
dreadful code whilst the 'char *te = "string"' generates much better code thus
if anything we should change all occurrences of 'char (.*)\[\] =' to "char *\1"
instead...
If you think about it logically it actually makes sense, too. In the 'char
te[] = "string"' case we are telling the compiler to take a constant string
"string" which will be stored both in the binary and once loaded in memory,
then make a copy of it into the array "te" and that is exactly what the
compiler does (if the string is short enough the compiler actually copies
immediate values instead of storing the string elsewhere but that is still
worse than just pointing to constant memory). Whilst in the 'char *te =
"string"' case we are telling the compiler to take a constant string (as
previous case) and then assign the address of that string to the pointer
variable "te". Thus no copying takes place. Thus one should never use the
'char te[] = "string"' form UNLESS you want to then mess about with the
contents of the string in which case modifying "te[]" will work as it is a
local copy but modifying "*te" will cause a crash or be ignored depending on
whether you are programming in the kernel or user space because you are trying
to modify a read-only memory segment...
Best regards,
Anton
On 17 May 2014, at 13:25, Manuel Schölling <manuel.schoell...@gmx.de> wrote:
> Initializations like 'char *foo = "bar"' will create two variables: a static
> string and a pointer (foo) to that static string. Instead 'char foo[] = "bar"'
> will allocate only a declare a single variable and will end up in shorter
> assembly (according to Jeff Garzik on the KernelJanitor's TODO list).
>
> Signed-off-by: Manuel Schölling <manuel.schoell...@gmx.de>
> ---
> fs/ntfs/inode.c | 2 +-
> fs/ntfs/super.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index f47af5e..3975798 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -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/super.c b/fs/ntfs/super.c
> index 9de2491..eb2c195 100644
> --- a/fs/ntfs/super.c
> +++ b/fs/ntfs/super.c
> @@ -676,7 +676,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;
>
> --
> 1.7.10.4
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
University of Cambridge Information Services, Roger Needham Building
7 JJ Thomson Avenue, Cambridge, CB3 0RB, UK
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
