Problem with Sprintf

ramkrix · October 20, 2010, 4:18pm

Hi,

I have the below sample code to hash the input number read from file. File will have 16 to 19 digit number and executable hash the number using some logic and returns the hashed value. Each digit in the 16 digit number is converted to a 4 byte value. That if the input is 16digit integer, the output is 64byte string. I have split the code in to two. One main function and other file having this hashing function as shown below:

MAIN:



ganesh@ubuntu:~/my_programs/c$ cat main.c
# include "fnhash.h"
# include <stdio.h>

int main(int argc, char *argv[])
{
	if(argc!=2)
	{
		printf("Usage wrong");
		exit(1);
	}

	FILE *fp;
	if((fp=fopen(argv[1],"r"))==NULL)
	{
		printf("File opening error\n");
		exit(1);
	}

	char *instr;
	char hashnum[100];

	while(!feof(fp))
	{
		while(fscanf(fp,"%s",instr)>0)
		{
			printf("in Number Length : %lu\n",strlen(instr));
			instr[strlen(instr)+1]='\0';
			printf("in Number = %s\n",instr);
			fnhash(instr,hashnum);
			printf("in number : %s\tHashed value : %s\n",instr,hashnum);
		}
	}

	return 0;
}

FUNCTION:


ganesh@ubuntu:~/my_programs/c$ cat fnhash.c
# include <stdio.h>
# include <stdlib.h>
# include <string.h>
# include "fnhash.h"

void fnhash(const char nbr[],char hsh[])
{
	char *tmpnbr, *tmphsh;
	tmpnbr=nbr;
	tmphsh=hsh;
	char a;
	char b;
	while(*tmpnbr!='\0')
	{
		a=('A'-*tmpnbr)+'A';
		b=('z'+*tmpnbr)-'A';
		sprintf(tmphsh,"%d%c%c",*tmpnbr,a,b);
		tmpnbr++;
		tmphsh=tmphsh+4;
	}
	printf("strlen of hashed value = %lu\n",strlen(hsh));
	tmphsh[strlen(tmphsh)+1]='\0';
	return;
}

The file having the numbers is as below:

ganesh@ubuntu:~/my_programs/c$ cat numbers.lst
1234567890123456
12345678901234567
1234567890123456789
123456

Execution Step as follows:

ganesh@ubuntu:~/my_programs/c$ make -f hashmake
gcc -Wall -g -c main.c
main.c: In function �main':
main.c:9: warning: implicit declaration of function �exit'
main.c:9: warning: incompatible implicit declaration of built-in function �exit'
main.c:16: warning: incompatible implicit declaration of built-in function �exit'
main.c:26: warning: implicit declaration of function �strlen'
main.c:26: warning: incompatible implicit declaration of built-in function �strlen'
main.c:26: warning: �instr' may be used uninitialized in this function
gcc -Wall -g main.o fnhash.o -o main

ganesh@ubuntu:~/my_programs/c$ ./main numbers.lst
in Number Length : 16
in Number = 1234567890123456
Segmentation fault (core dumped)

On Analysis with GDB,

ganesh@ubuntu:~/my_programs/c$ gdb main core
GNU gdb (GDB) 7.1-ubuntu
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/ganesh/my_programs/c/main...done.
[New Thread 3605]

warning: Can't read pathname for load map: Input/output error.
Reading symbols from /lib/libc.so.6...(no debugging symbols found)...done.
Loaded symbols for /lib/libc.so.6
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
Core was generated by `0645Uf49Qj49Qj56Jq'.
Program terminated with signal 11, Segmentation fault.
#0  0x00007f578f8fbb87 in vsprintf () from /lib/libc.so.6

What is the possible reason for this error?? Where Am I going wrong??

Thanks,
Ramkrix

DGPickett · October 20, 2010, 4:34pm

Well, vsprintf, not sprintf, and the code for vsprintf is not here. As I recall, vsprintf is very sensitive to proper usage! (I had a problem with one old Sun compiler inlining away call that generated the stack that had my vsprintf arguments!) Being a 64 bit compile adds another factor I am not sure of, but hopefully everyone is in agreement about the size of stack frames. You might add the includes and code changes to make the warnings go away.

Your sprintf says %d but you give a char* not an int.

Corona688 · October 20, 2010, 4:41pm

char *instr;

I think you made it a pointer type since the functions said they needed a pointer, but you only got half the idea. sprintf and fnhash take pointers which point to what memory you want them to use. The memory doesn't create itself, so all you're giving them is a pointer to nothing. the commands mindlessly obey the command to write to invalid memory and cause a segmentation fault, hence why the crash is there despite the bug being far earlier.

char instr[100];

This way when you pass 'instr' it will point to the 100 bytes of space it's defined as. Be careful not to go beyond it.

DGPickett · October 20, 2010, 5:01pm

PS: I usually pass 'char *x' not 'char x[]' as it is more truthful and you should not be moving arrays by value!

For instance, int main( int args, char *argv ) says what is in memory on the stack. argv is a pointer needing double dereferencing to find a character. We better hope that the first dereference gives us (argv points to) an array of char somewhere in memory. Actually, argc is derived from the count of elements in that array up to the first null (0) pointer, see "man 2 exec", so it will be argc+1 elements long. The caller, lets say exec(), created it before it called main(). The pointers in that array point to additional areas where the null terminated strings of the arguments passed to exec() are stored. So, for a command with 3 arguments, there are 6 memory areas in play: 4 strings, a char * array of 5 pointers, and a stack frame of, for 32 bit code, an int and a pointer (to a pointer to a character). The stack frame is created by the call, but all the rest had to be provided by code: allocated (malloc or declare) and loaded with pointers, data bytes and nulls.

If you are in a hurry, calloc() does the multiply, of size and count, and clearing to nulls = zeroes for you.

Corona688 · October 20, 2010, 5:22pm

It's passed by reference either way.

#include <stdio.h>

void function(char arr[60])
{
        printf("arr=%p\n", arr);
}

int main(void)
{
        char buf[60];

        printf("buf=%p\n", buf);
        function(buf);
}

 $ ./a.out
buf=0xbf8d06c8
arr=0xbf8d06c8
$

All giving a function parameter an array type does is stop code in the function from modifying the pointer value. Considering making a pointer copy they can modify is the first thing done in this function though, they might as well make it a pointer and be done with it.

ramkrix · October 21, 2010, 1:42am

Thank you Corona and DGpicket. As mentioned, I allocated instr with instr[100] and it workd now.. Thank you:)

---------- Post updated at 11:12 AM ---------- Previous update was at 11:08 AM ----------

One more question. How to eliminate the warnings here? I have included all necessary header files in "fnhash.c" and the below is my makefile:

ganesh@ubuntu:~/my_programs/c$ cat hashmake
main : main.o fnhash.o
	gcc -Wall -g main.o fnhash.o -o main
main.o : fnhash.h main.c
	gcc -Wall -g -c main.c
fnhash.o : fnhash.c fnhash.h
	gcc -Wall -g -c fnhash.c

However I'm getting those warnings.
Thanks,
Ramkrix

DGPickett · October 21, 2010, 9:46am

Yes, never lie to yourself, especially!

corona688:

It's passed by reference either way.
#include <stdio.h>

void function(char arr[60])
{
   printf("arr=%p\n", arr);
}

int main(void)
{
   char buf[60];

   printf("buf=%p\n", buf);
   function(buf);
}
 $ ./a.out
buf=0xbf8d06c8
arr=0xbf8d06c8
$
All giving a function parameter an array type does is stop code in the function from modifying the pointer value. Considering making a pointer copy they can modify is the first thing done in this function though, they might as well make it a pointer and be done with it.

---------- Post updated at 09:27 AM ---------- Previous update was at 09:09 AM ----------

Type warnings:

You should match your subroutines to prototypes, usually by using the #include lines in man 2 xxx, or for main, explicitly say it returns int. For instance, with #include <strings.h>, strchr() is know to return char* not the default int, which is a type violation.

For your own subroutines, prototype them up top, if linked in your own #include "my_include_make_dir_or_minus_I_dir_relative_path_to_file". If they are in this file, you can use my favorite PASCAL cheat: declare/define them above main() or above wherever they are called, so you do not have to sync the prototype to any argument mods.

Remember boolean is int, so for instance, when you check to ensure your malloc succeeded:

if ( !(cptr=malloc(...)))

should be:

if ( !(int)(cptr=malloc(...)))

or:

if ((char*)NULL != (cptr=malloc(...)))

(I like putting the smaller and more constant operand on the left for easy reading.)

ramkrix:

Thank you Corona and DGpicket. As mentioned, I allocated instr with instr[100] and it workd now.. Thank you:)

---------- Post updated at 11:12 AM ---------- Previous update was at 11:08 AM ----------

One more question. How to eliminate the warnings here? I have included all necessary header files in "fnhash.c" and the below is my makefile:
ganesh@ubuntu:~/my_programs/c$ cat hashmake
main : main.o fnhash.o
   gcc -Wall -g main.o fnhash.o -o main
main.o : fnhash.h main.c
   gcc -Wall -g -c main.c
fnhash.o : fnhash.c fnhash.h
   gcc -Wall -g -c fnhash.c
However I'm getting those warnings.
Thanks,
Ramkrix

For instance, my "man 2 exit" says exit() needs: #include <stdlib.h>