5

Hi all I trying to assign string into a char * pointer. Below is how I am doing but I am getting this warning: assignment makes integer from pointer without a cast.

What is the correct why to deal with strings in C ?

char* protoName = "";

if(h->extended_hdr.parsed_pkt.l3_proto==0)
    *protoName = "HOPOPT";  
else if(h->extended_hdr.parsed_pkt.l3_proto==1)
    *protoName = "ICMP";    
else if(h->extended_hdr.parsed_pkt.l3_proto==2)
    *protoName = "IGMP";    
else if(h->extended_hdr.parsed_pkt.l3_proto==3)
    *protoName = "IGMP";    
else if(h->extended_hdr.parsed_pkt.l3_proto==4)
    *protoName = "IGMP";

char all[500];

sprintf(all,"IPv4','%s'",* protoName);
Improve this question
5
  • *protoName is the same as protoName[0]. A single character will not hold a string. So use protoName = "HOPOPT"; Commented Aug 28, 2013 at 15:47
  • @So then which is the best way to change it Commented Aug 28, 2013 at 15:51
  • *protoName equals protoName[0]. That's a single character. You want to change the place protoName points to. So say protoName = "abcd"; Commented Aug 28, 2013 at 15:54
  • So the first char* protoName define it as a pointer to list of char is it ? IT can be single or multiple char right ? Commented Aug 28, 2013 at 15:56
  • char *protoName defines a pointer to a character, called protoName. There's no such thing as a "list of char". The pointer can point to a single char, it can point to an array of char, it can point to a string and for all those combinations, it can be pointing to memory on stack or the heap. It can also point to junk, but that's an answer for another question. Commented Aug 28, 2013 at 16:01

5 Answers 5

Reset to default
8

If you just want to change which string literal protoName points to, you just need to change

*protoName = "HOPOPT";

to

protoName = "HOPOPT";

*protoName = attempts to write to the first character pointed to by protoName. This won't work in your case as protoName points to a string literal which cannot be modified.

You also need to change your sprintf call to

sprintf(all,"IPv4','%s'", protoName);

The %s format specifier signals that you'll be passing a pointer to a nul-terminated char array. *protoName gives you the character code of the first character pointed to by protoName; sprintf doesn't know this so would treat that character code as the address of the array to read from. You don't own this memory so the effects of reading from it would be undefined; a crash would be likely.

As an aside, if you had a writeable char array and wanted to change its contents, you'd need to use strcpy to copy a new array of chars into it.

Improve this answer
Sign up to request clarification or add additional context in comments.

21 Comments

How about me putting to the sprintf is it correct to put * protoName
@user2711681 I'd missed that. I've updated my answer to explain that this line needs to change too
@So what is the writeable char array ? Isnt this *protoName a dynamic pointer to a list of char ?
A writeable char array is one where you've allocated your own memory rather than just a pointer to someone else's memory. char str[20] or char* str = malloc(20) would give you a writeable array with space for 20 chars.
@user2711681 No, protoName is a pointer to a string literal (which cannot be modified and should be treated as constant that is empty). *protoName is an attempt to dereference the pointer protoName and will give you the first thing that protoName points to. In this case, a char which will be 0. I won't pussy-foot around: if you plan on working with C you MUST understand the differences between arrays, pointers and strings, what the address-of '&' and dereference '*' operators do and how memory allocation works.
|
2

If you are using constants, then simply reassign the pointer, not the contents:

const char* protoName = "";
if(h->extended_hdr.parsed_pkt.l3_proto==0)
    protoName = "HOPOPT";    
else if(h->extended_hdr.parsed_pkt.l3_proto==1)
    protoName = "ICMP";  
else if(h->extended_hdr.parsed_pkt.l3_proto==2)
    protoName = "IGMP";  
else if(h->extended_hdr.parsed_pkt.l3_proto==3)
    protoName = "IGMP";  
else if(h->extended_hdr.parsed_pkt.l3_proto==4)
    protoName = "IGMP";
Improve this answer

4 Comments

@Why I need to define as *protoNAme but assign is just protoNAme what is the difference ?
Because that's how C works. It is the same as your initial line, char *a = b;, and on every other operation you don't need to add char * all the time. It's exactly the same as int x = 1; x = 2; -- for your string, the type is char *.
@user2711681 You are confused because the * operator has three meanings in C: it can mean "multiply", it can mean "declare as pointer" or it can mean "dereference pointer" and stumbling on the fact that strings are handled "specially" in C.
@Yes u are right I am confuse with * especially in the pointer area ?
2

Here's some examples:

#include <stdio.h>

const char * protoNameFromPktId(int id) {
    static char* protoName[] = { "HOPOPT", "ICMP", "IGMP", "IGMP","IGMP"};
    return protoName[id];
}

main() {
   printf("%s\n", protoNameFromPktId(2));
   char all[500];
   sprintf(all,"%s", protoNameFromPktId(2));
   strcpy(all, protoNameFromPktId(2));
}
Improve this answer

7 Comments

@What is the first index 0 or 1 in this case ? What is the speed difference between this and if else ?
It is zero. It matches your if - else.
@how about efficiency in comparison to using if else ?
The if else is slower than the array indexing, but putting the array index in a function call probably loses any advantage. Maybe the compiler will inline that, and maybe it won't, but you can define the array in your function, as given (make is const static char *protoName[], though) and using protoName[extended_hdr.parsed_pkt.l3_proto] is just as easy and self-explanatory as protoNameFromPktId(extended_hdr.parsed_pkt.l3_proto).
PS: unsigned id=extended_hdr.parsed_pkt.l3_proto; and then char *proto=(id < sizeof protoName / sizeof *protoName) ? protoName[id] : ""; will guard against invalid or unexpected protocol values--just as your if/else if chain does now.
|
1

You just need to do this:-

protoName = "HOPOPT";

instead of 

*protoName = "HOPOPT";

So change like :-

char* protoName = "";
  if(h->extended_hdr.parsed_pkt.l3_proto==0)
  protoName = "HOPOPT";    
  else if(h->extended_hdr.parsed_pkt.l3_proto==1)
  protoName = "ICMP";  
  else if(h->extended_hdr.parsed_pkt.l3_proto==2)
  protoName = "IGMP";  
  else if(h->extended_hdr.parsed_pkt.l3_proto==3)
  protoName = "IGMP";  
  else if(h->extended_hdr.parsed_pkt.l3_proto==4)
  protoName = "IGMP";

  char all[500];
    sprintf(all,"IPv4','%s'",* protoName);
Improve this answer

Comments

1

The main problem that I can see is that you dont bind memory for the strings, so you have to use malloc or better use strdup function that allocates memory automatically. because if you assigned big strings then you should have a problem. The problem with the warning answered by the others, so is ok. plz correct me if I'm wrong.

char* protoName;
if(h->extended_hdr.parsed_pkt.l3_proto==0){
   protoName = strdup("HOPOPT");    
}
else if(h->extended_hdr.parsed_pkt.l3_proto==1){
  protoName = strdup("ICMP");  
...
char all[500];
sprintf(all,"IPv4','%s'",* protoName);
free(protoName);
Improve this answer

4 Comments

Don't forget free(protoName);. However what is the point in copying a constant string?
Note that you'd also need a final else block with this approach to avoid the possibility of an uninitialised protoName being dereferenced by sprintf.
It's perfectly valid to do char *protoName = ""; protoName = "ICMP"; just as long as you understand what it is the code does, what protoName points to and how it can used.
Why is the free needed here ?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.