Beginner: help with pointer and memory

[thinICE80]

I wrote this, works nearly as expected but on the last element extracted from input string it attach various unexpected character, please any suggestion??
In particular look at divCmd function.

#include <stdio.h>
#include <unistd.h>
#include <string.h>

#define BUFLEN 1024
#define STDIN 0
#define STDOUT 1

char ** divCmd(char *);

int main(int argc, char *argv[])
{
char cmd[BUFLEN];
char shellTxt[9] = "myShell$ ";
int nByte;
char **prova;

write(STDOUT,shellTxt,9);
nByte = read(STDIN,cmd,BUFLEN);
//write(STDOUT,buf,nByte);
prova = divCmd(cmd);
puts(prova[2]);
}

char ** divCmd(char *cmd)
{
char *cmdArr[50];
char **arr;
int i;

cmdArr[0] = strtok(cmd," ");
printf("cmdArr[0] = %s\n", cmdArr[0]);
for(i=1; i<50; i++)
{
cmdArr = strtok(NULL," ");
if(cmdArr != NULL)
{
printf("cmdArr[%d] = %s\n", i, cmdArr);
} else break;
}
return cmdArr;
}

 

[TrojanWarBlade]

You are probably missing a null terminator from the end of your "cmd".
Try adding "cmd[nByte] = '\0';" after your "read" call.



Trojan.

 

[thinICE80]

Thanks, works fine if I add \0 at nByte-1.

cmd[nByte-1]='\0';

 

[TrojanWarBlade]

I'm sorry but you should not do that. You should do what I suggested.
Lets say the "read" produced "string" (6 bytes), they would be in an array indexed 0 thru 5 and you'd want the null terminator at char 6 (the LENGTH).
I did not miss out the "-1" by accident.
If you put the null char at nByte-1 you'd overwrite the last byte.


Trojan.

 

[SamBones]

Also, You haven't allowed enough room for [tt]shellTxt[/tt]. There's a null terminator.

You need to change...

[tt]char shellTxt[9] = "myShell$ ";[/tt]

...to either...

[tt]char shellTxt[10] = "myShell$ ";[/tt]

...or...

[tt]char shellTxt[] = "myShell$ ";[/tt]

Hope this helps.

 

[thinICE80]

I fix the room issue for ShellTxt, also if it does not creat problem. Thanks SamBones.

@Trojan
Yes, probably I overwrite the last char but is what I want, I think that the read function add also a \n to the buffer and don't close the string with \0.

So "cmd[nByte-1]='\0';" overwright the \n with \0 and output looks like:

myShell$ qui quo qua paperino pluto
cmdArr[0] = qui
cmdArr[1] = quo
cmdArr[2] = qua
cmdArr[3] = paperino
cmdArr[4] = pluto
qui

Thanks again Trojan.

 

[TrojanWarBlade]

So long as you understand that you are overwriting the last character (as it appears you do) then that's fine.
Hopefully the null character has solved your problems.



Trojan.

 

[mingis]

Before doing

cmd[nByte-1]='\0';

check whether nByte is not zero (for example in case of empty input file). There could be no '\n' at the end of the line too - check for presence of it at cmd[nByte-1].

 

[ArkM]

Some addition:
I think that 1/2 of thinICE80's troubles arises from unproper input method choice. Avoid using of non-standard binary file i/o routines read/write. Use ANSI standard fread (or better fgets in that case) from <stdio.h>: fgets adds string zero terminator and controls buffer size.

It's nonsense to use binary input on text file (and it's nonsense to add zero terminator then use strtok on binary data...

 

[thinICE80]

@ArkM

Yes, I was thinking the same, someone from school say me to use read and write, but I noticed that these function are really out of scope.
On the other hand using these I had to understand well pointers.

 

[thinICE80]

I tried this as input...

#include <stdio.h>

int main(int argc, char *argv[])
{
char *input = (char *)malloc(1000*sizeof(char));

gets(input);
printf("%s",input);
}

...looks for me simpler and works fine, but dunno why compiler tells me that gets is a dangerous function and should not be used, boh!

 

[TrojanWarBlade]

Because "gets" will blindly keep writing data to your buffer even if your buffer runs out of space. "gets" will overshoot the and and then continue scribbling over memory space that it has no right to access.
The usual way around that is to create a buffer so large that there is virtually no chance at all that you would ever process a line that long. 16K, 32K, 64K buffers are not unusual to minimise any risk.


Trojan.

 

[ArkM]

Use fgets(buffer,length,file) in that case. The only discomfort: don't forget to replace newline char '\n' if you need (fget replaces it with '\0', fgets does not).
But it's so simple:

char* Fgets(char* str, int n, FILE* stream)
{
  char* p;
  p = fgets(str,n,stream);
  if (p)
  {
    n = strlen(str) - 1;
    if (n >= 0 && str[n] == '\n')
       str[n] = '\0';
  }
  return p;
}