0

Been playing with itoa() for a school project and it was working fine then started to throw errors. Says its having a segmenation error when first instance of itoa is handled.

Here's the offending code.

I don't see why it would work at first then start having issues. The only thing I had added pre-breakdown was some lines of printf() at the bottom which I didn't include as I've already commented them out of the code and it still doesn't work.

#include <stdio.h>
#include <stdlib.h>

int main()
{

//Variables

unsigned int byteH=0b00011001;
unsigned int byteL=0b00001110;
char* sValue;
char* sFreq;
float iConv;
char Output[4];
int i;


i=((byteH*32)+byteL);  // just adding two 5bit blocks together

itoa(i,sValue,10);     // This instance throws the segmenation error

iConv=((byteH*32)+byteL);
iConv=(int)(iConv/1.023);
i=(int)iConv;

itoa(i,sFreq,10);     // This instance doesn't cause problems.
Chef Flambe
  • 885
  • 2
  • 15
  • 35
  • The reason it might "work" and then not work is because the pointers were not set to point to storage. Uninitialized values are "random". As in, not truly random but dependent on complex factors that aren't worth your time to figure out much like a pseudo-random number generator. – Zan Lynx Feb 24 '13 at 22:30

2 Answers2

4

The function itoa expects an alreay allocated buffer. Try:

char* sValue = malloc(20);

Same goes for sFreq, even if it happens to "work" as it is.

cnicutar
  • 178,505
  • 25
  • 365
  • 392
  • Awesome! Thanks, totally fixed the issue. – Chef Flambe Feb 24 '13 at 22:08
  • 1
    @ChefFlambe Don't forget to `free(sValue)` once you're done with it. – cnicutar Feb 24 '13 at 22:08
  • Given the original error, don't you think that using malloc/free is perhaps inviting just a different kind or error. The more obvious and safer (and in fact more efficient) solution would be simply to declare sValue as an array: `char sValue[21]`. Note that 21 characters are required to accommodate the string for a maximum length 64 bit unsigned with nul terminator. On a 32 bit platform the array needs to be only 11 characters. – Clifford Feb 24 '13 at 22:31
  • Probably a reasonable response ;-) – Clifford Feb 24 '13 at 22:50
  • @Clifford Your suggestion is valid and sensible but I'm too lazy to edit plus I don't think it will make a difference until the OP advances further in C and learns *on his own* to avoid `malloc` where possible. But have an upvote on your answer since you mention `sprintf` (you could have mentioned `snprintf` though). – cnicutar Feb 24 '13 at 22:52
  • When learning to code, I'd expect to learn about arrays which are built in to the language before learing about dynamic memory which isn't and needs library support. BTW, I was not paying attention, i is not unsigned, the array needs also to acommodate a potential sign prefix. – Clifford Feb 24 '13 at 22:59
1

While itoa() is defined as taking a pointer argument, that pointer must point to a memory block suficiently large to accept the result. In your case the pointers are unitialised.

The simplest solution is to declare suitably sized arrays. Arrays can be passed to functions accepting pointer arguments - they are passed by reference, so the function receives a pointer to the array, not a copy of it.

char sValue[22] ;
char sFreq[22] ;

The size of the array is sufficient to accept a 64 bit integer with a potential sign prefix. It need be only 12 for 32 bit, but you may as well ensure you don't have to know what the code will be compiled for.

An alternative to itoa would be to use sprintf:

sprintf( sValue, "%u", i ) ;
Clifford
  • 88,407
  • 13
  • 85
  • 165