problem with memmove in C

[rajanandhini]

Hi,

I have written a small program using memmove. I get memory fault while executing the executable. Please let me know if there are any errors and help me in this regard.
The program is as below

#include
#include
#include

int main()
{
char *fn;
strcpy(fn,"tape");
printf("Value in fn is %c", *fn);
memcpy(fn+1,fn,(strlen(fn) + 1));
printf("Value in fn+1 is %c", *fn+1);
return 0;
}

We analysed the core dump and we are getting the following error

Program terminated with signal 11, Segmentation fault.
SEGV_ACCERR - Invalid Permissions for object
#0 0x60000000c01d7a70:0 in strcpy+0x250 () from /usr/lib/hpux32/libc.so.1
(gdb) bt
#0 0x60000000c01d7a70:0 in strcpy+0x250 () from /usr/lib/hpux32/libc.so.1
#1 0x4000950:0 in main () at memtest.c:8


Thanks,
nan

[Steven Schweda · Accepted solution]

"char *fn;" says that "fn" points to some
character storage, but you need to supply
that storage, and you need to point "fn" to
it. You've done neither, so "fn" is probably
a null pointer, and so strcpy() fails when
you try to use it.

One solution:

char fn[33]; /* Or some other number. */

What, exactly, are you trying to do with that
memcpy()? What, exactly, are you trying to
do with any of this?

With a serious compiler:

alp $ cc xx

strcpy(fn,"tape");
^
%CC-W-UNINIT1, The scalar variable "fn", declared in "main", is fetched but not
initialized. And there may be other such fetches of this variable that have not
been reported in this compilation.
at line number 8 in file ALP$DKA0:[SMS]XX.C;1

alp $ cc /version
HP C V7.1-015 on OpenVMS Alpha V7.3-2

[Steven Schweda]

And a "\n" at the end of each printf() format
string would probably help the appearance of
the output, too.

[Dennis Handly]

#include

This header is non-Standard and you should remove it.

>Steven: With a serious compiler:

Right, what version of the compiler are you using? aCC6 gives:
warning #2549-D: variable "fn" is used before its value is set
strcpy(fn,"tape");

To prevent you from making mistakes like this, you can use +We2549 to make it an error.

memcpy(fn+1,fn,(strlen(fn) + 1));

This code is illegal. You are not allowed to use memcpy if the operands overlap, you must use memmove(3).

printf("Value in fn+1 is %c", *fn+1);

This is probably not what you want. You should use: fn[1]
And you could use fn[0] for your first one.

[rajanandhini]

Thanks a lot for your assistance. Now I am able to compile the code successfully with your advice. I just have few doubts using malloc.

1. Is it necessary that we should set the memory with 0 bytes before using malloc?
2. Is it mandatory that we allocate memory as below if the pointer is a character pointer?
fn=(char*)malloc(10);

Or, can we just allocate as fn=malloc(10), where fn is a character pointer.

[Steven Schweda]

> This is probably not what you want. [...]

I doubt that any of us knows what is wanted
here, but for more fun, you might also try
adding a line like:

printf("Value in (fn+1) is %c\n", *(fn+1));

Although it is interesting that 't'+1 = 'u'.

Do enough of this, and you may learn some C.

[Steven Schweda]

1. Set _what_ memory? Before you use
malloc(), you don't have any memory to set.

2. No, but why trust me? Ask the compiler.

[Dennis Handly]

>1. Is it necessary that we should set the memory with 0 bytes before using malloc?

That's up to you. If you need that, you can use calloc(3).

>2. Is it mandatory that we allocate memory as below if the pointer is a character pointer?
fn=(char*)malloc(10);

Yes. If you want to use strict type checking and for sure for C++

>can we just allocate as fn=malloc(10), where fn is a character pointer.

Since malloc returns a void*, you should really cast to a char*. And you MUST for C++. It is a good habit to do that.

[Matti_Kurkela]

1.) It depends on your program. If your program is guaranteed to always write to each byte of the malloc'ed memory before it reads that byte, then you can certainly skip the initialization.

Of course, if you're wrong about this quality of your program, you may get strange results and it may take a long time to figure out why.

Note that you could also use calloc() instead of malloc(): it allocates the memory just like malloc() and then automatically fills the newly-allocated memory with zero bytes.

Because calloc() needs to do more work, it's not quite as fast as malloc(), so if you're writing a section of program that must execute absolutely as fast as possible, you might have to think about the difference.

2.) malloc() (and calloc()) will return a "pointer to void" (void *). Your variable is set to receive a "pointer to char" (char *). The compiler may change pointers to void automatically to other pointer types, but if you have compiler warnings enabled, it might say something like "warning: implicit cast ..." if you write it as "fn=malloc(10);".

It's good style to make type casts like this explicit, like "fn=(char*)malloc(10);". It also means you get fewer useless warnings from the compiler.


Another style point: my C teacher used to say it's bad to embed "magic constants" (like the size of the malloc'ed memory in your example) inside your program code. It's much better to explicitly #define a name for that constant in the beginning of the file and then use that name _everywhere_ you need to refer to that constant.

So, your example might look more like this:

#include ...
#define FN_MAXLEN 10

...

fn=(char*)malloc(FN_MAXLEN);

If you do it this way, you'll only need to change the number in one place if you need to make the malloc'ed memory area bigger.

If you don't do this, you have to read through your code to find all references to the size of memory pointed by fn, and change each of them one by one. If you miss one of the references, your change has introduced a bug.

Another advantage of using named constants is that if you chose the names well, your program will be easier to understand and maintain.

MK

[rajanandhini]

Thanks a lot. It was informative. Thanks for your assistance.

[Sandman!]

Since the "tape" is of type array of char you cannot to initialize a pointer to it. In fact as mentioned before you can declare and initialize a char array to hold it. However that approach wastes memory so you can simply equate the char pointers. Moreover it isn't clear from your posted source code what you are trying to accomplish? Are you trying to walk along the string "tape" or trying to increment the char that fn points to? Because depending on the result the desired code can be tweaked as in the program below which assumes that you are trying to walk along the string "tape". In fact the code prints out both the outcomes; first one assuming that you want to walk along the string "tape" and second one assuming that you want to increment the value of the char being pointed to:

#include
#include
#include

int main(void)
{
char *fn;

fn = "tape"; /* equate the char pointers */
printf("Value in fn is %c\n", *fn); /* print the char that fn points to */
printf("Value in fn+1 is %c\n", *(fn+1)); /* walk along the string "tape" */
printf("Value in fn+1 is %c\n", *fn+1); /* increment the char fn points to */
return 0;
}

[rajanandhini]

Thanks for the information.

[rajanandhini]

I have got the required solution. Thanks

[Dennis Handly]

>Sandman:
char *fn;
fn = "tape"; /* equate the char pointers */

fn should be declared as const char*. You should not be trying to modify characters in the string "tape".