1

I am having trouble deciphering the Valgrind output that I am getting. For example I have an error that says I am doing an illegal write of size 1. So I added one to the malloc and that doesn't change anything. Since I have never used Valgrind before, I thought it would be best to get some help after searching online yielded no responses. Please be aware that I am not looking for the answer so I have not reproduced my code. I would like to be pushed in the correct direction so that I can combat future errors. Thanks!

==28881== Memcheck, a memory error detector ==28881== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al. ==28881== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info ==28881== Command: ./sws root ==28881== ==28892== Conditional jump or move depends on uninitialised value(s) ==28892== at 0x4C286D9: __GI_strlen (mc_replace_strmem.c:284) ==28892== by 0x4040B7: get_headers (html.c:280) ==28892== by 0x402D77: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Conditional jump or move depends on uninitialised value(s) ==28892== at 0x4C286D9: __GI_strlen (mc_replace_strmem.c:284) ==28892== by 0x4040C5: get_headers (html.c:280) ==28892== by 0x402D77: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Conditional jump or move depends on uninitialised value(s) ==28892== at 0x4C286D9: __GI_strlen (mc_replace_strmem.c:284) ==28892== by 0x4040D3: get_headers (html.c:280) ==28892== by 0x402D77: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Conditional jump or move depends on uninitialised value(s) ==28892== at 0x4C286D9: __GI_strlen (mc_replace_strmem.c:284) ==28892== by 0x4040E1: get_headers (html.c:280) ==28892== by 0x402D77: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid write of size 1 ==28892== at 0x4C2874C: strcpy (mc_replace_strmem.c:311) ==28892== by 0x4041E1: get_headers (html.c:299) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid write of size 1 ==28892== at 0x4C2875F: strcpy (mc_replace_strmem.c:311) ==28892== by 0x4041E1: get_headers (html.c:299) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be034 is 4 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid read of size 1 ==28892== at 0x4C28374: strcat (mc_replace_strmem.c:176) ==28892== by 0x4041F7: get_headers (html.c:300) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid write of size 1 ==28892== at 0x4C2838C: strcat (mc_replace_strmem.c:176) ==28892== by 0x4041F7: get_headers (html.c:300) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be034 is 4 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid write of size 1 ==28892== at 0x4C2839F: strcat (mc_replace_strmem.c:176) ==28892== by 0x4041F7: get_headers (html.c:300) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be061 is 15 bytes before a block of size 40 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x403E53: get_headers (html.c:224) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid read of size 1 ==28892== at 0x4C28374: strcat (mc_replace_strmem.c:176) ==28892== by 0x40420D: get_headers (html.c:301) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== Invalid read of size 1 ==28892== at 0x4C28374: strcat (mc_replace_strmem.c:176) ==28892== by 0x40420D: get_headers (html.c:301) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid write of size 1 ==28892== at 0x4C2838C: strcat (mc_replace_strmem.c:176) ==28892== by 0x40420D: get_headers (html.c:301) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be061 is 15 bytes before a block of size 40 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x403E53: get_headers (html.c:224) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid read of size 1 ==28892== at 0x4C28374: strcat (mc_replace_strmem.c:176) ==28892== by 0x404223: get_headers (html.c:302) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid read of size 1 ==28892== at 0x4C28374: strcat (mc_replace_strmem.c:176) ==28892== by 0x404239: get_headers (html.c:303) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid write of size 1 ==28892== at 0x4C2838C: strcat (mc_replace_strmem.c:176) ==28892== by 0x404239: get_headers (html.c:303) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be098 is 0 bytes after a block of size 40 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x403E53: get_headers (html.c:224) ==28892== by 0x402E00: read_page (networking.c:347) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Invalid read of size 1 ==28892== at 0x4C286E4: __GI_strlen (mc_replace_strmem.c:284) ==28892== by 0x402E0F: read_page (networking.c:348) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== Syscall param socketcall.sendto(msg) points to unaddressable byte(s) ==28892== at 0x5121052: send (send.c:28) ==28892== by 0x40276D: send_msg (networking.c:245) ==28892== by 0x402E29: read_page (networking.c:348) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== Address 0x53be030 is 0 bytes after a block of size 32 alloc'd ==28892== at 0x4C274A8: malloc (vg_replace_malloc.c:236) ==28892== by 0x402D84: read_page (networking.c:341) ==28892== by 0x402B8E: get_page (networking.c:310) ==28892== by 0x40268E: header_parse (networking.c:232) ==28892== by 0x401D96: retrieve_msg (networking.c:62) ==28892== by 0x401AAB: main (sws.c:139) ==28892== ==28892== ==28892== HEAP SUMMARY: ==28892== in use at exit: 337 bytes in 10 blocks ==28892== total heap usage: 19 allocs, 9 frees, 3,307 bytes allocated ==28892== ==28892== LEAK SUMMARY: ==28892== definitely lost: 337 bytes in 10 blocks ==28892== indirectly lost: 0 bytes in 0 blocks ==28892== possibly lost: 0 bytes in 0 blocks ==28892== still reachable: 0 bytes in 0 blocks ==28892== suppressed: 0 bytes in 0 blocks ==28892== Rerun with --leak-check=full to see details of leaked memory ==28892== ==28892== For counts of detected and suppressed errors, rerun with: -v ==28892== Use --track-origins=yes to see where uninitialised values come from ==28892== ERROR SUMMARY: 348 errors from 17 contexts (suppressed: 4 from 4) ==28881== ==28881== HEAP SUMMARY: ==28881== in use at exit: 0 bytes in 0 blocks ==28881== total heap usage: 0 allocs, 0 frees, 0 bytes allocated ==28881== ==28881== All heap blocks were freed -- no leaks are possible ==28881== ==28881== For counts of detected and suppressed errors, rerun with: -v ==28881== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4) 
217 int get_headers(char* time, char* last_modified, char* server, char* content_type, size_t msglen, char* header, int flag) 218 { 219 char* header1 = NULL; 220 char* header2 = NULL; 221 char* header3 = NULL; 222 char* header4 = NULL; 223 char* header5 = NULL; 224 225 if ((header1 = (char*)malloc(strlen("Date: ")+strlen(time)+2) ) ==NULL) 226 { 227 fprintf(stderr, "%s: No more memory\n", getprogname()); 228 exit(EXIT_FAILURE); 229 } 230 231 232 strcpy(header1, "Date: "); 233 strcat(header1, time); 234 strcat(header1, "\n"); 235 236 237 if ((header2 = (char*)malloc(strlen("Last-Modified: ")+strlen(last_modified)+5) ) ==NULL) 238 { 239 fprintf(stderr, "%s: No more memory\n", getprogname()); 240 exit(EXIT_FAILURE); 241 } 242 243 244 strcpy(header2, "Last-Modified: "); 245 strcat(header2, last_modified); 246 strcat(header2, "\n"); 247 248 249 if ((header3 = (char*)malloc(strlen("Server: ")+strlen(server)+5) ) ==NULL) 250 { 251 fprintf(stderr, "%s: No more memory\n", getprogname()); 252 exit(EXIT_FAILURE); 253 } 254 255 256 strcpy(header3, "Server: "); 257 strcat(header3, server); 258 strcat(header3, "\n"); 259 260 261 if ((header4 = (char*)malloc(strlen("Content-Type: ")+strlen(content_type)+5) ) ==NULL) 262 { 263 fprintf(stderr, "%s: No more memory\n", getprogname()); 264 exit(EXIT_FAILURE); 265 } 266 267 268 strcpy(header4, "Content-Type: "); 269 strcat(header4, content_type); 270 strcat(header4, "\n"); 271 272 273 int content_length = strlen(header1)+strlen(header2)+strlen(header3)+strlen(header4)+strlen("Content-Length: ")+msglen+5; 274 char temp[10] = {' '}; 275 snprintf(temp, sizeof(msglen),"%d", content_length); 276 277 if ((header5 = (char*)malloc(strlen("Content-Length: ")+strlen(temp)+5) ) ==NULL) 278 { 279 fprintf(stderr, "%s: No more memory\n", getprogname()); 280 exit(EXIT_FAILURE); 281 } 282 283 284 strcpy(header5, "Content-Length: "); 285 strncat(header5, temp, strlen(temp)); 286 strcat(header5, "\n\n"); 287 288 289 if(flag == 1) 290 { 291 strcpy(header, header1); 292 strcat(header, header2); 293 strcat(header, header3); 294 strcat(header, header4); 295 strcat(header, header5); 296 } 297 298 return content_length; 299 free(header1); 300 free(header2); 301 free(header3); 302 free(header4); 303 free(header5); 304 }
Improve this question
9
  • 3
    Seeing the respective code locations would be helpful to explain those messages. Commented Nov 7, 2012 at 22:53
  • @BjoernD I've added the function (with relative line numbers) that problems with valgrind Commented Nov 7, 2012 at 23:00
  • Is it possible that flag is false? In that case you don't put any data into header1, header2 etc. but you still call strlen with them (uninitialised). Commented Nov 7, 2012 at 23:06
  • strcat(header1, "\0"); - this is just silly. it doesn't do anything. Commented Nov 7, 2012 at 23:16
  • and what's the point of those mallocs? you could directly write to header. Commented Nov 7, 2012 at 23:19

3 Answers 3

Reset to default
3

If flag is 0, then you never initialize any of the data in the header1..4 strings, so when you try to get their lengths with strlen, you read in uninitialized garbage. You should make sure to always initialize the header values to empty strings regardless of the flag setting.

Also, there's no need to strcat(..., "\0") onto the string -- strcpy and strcat always null-terminate the strings (in fact, "\0" is indistinguishable from the empty string "", since both of their first bytes are the NUL byte). And beware of Shlemiel the Painter algorithms when concatenating a bunch of strings together.

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

2 Comments

Based on OP's + 5 for the malloc, I have a strong suspicion that he really meant to write strcat(..., "\\n\\0");. I would use sprintf or something similar.
no, that's not what he meant. it's generating an HTTP response header
1

1) You malloc'd 5 pieces of memory and did not free them in your function. These should be freed before you return or exit. Notice how valgrind is saying you have 326 bytes that were definitely lost.

2) If flag is false, your header* variables never get set to anything, which is why valgrind is giving you a bunch of errors on line 280. Subsequently, there's nothing good for it to copy later around line 301, so you get more errors.

Improve this answer

Comments

1

Since you are interested in writing 'neat + cool' code, I am going to attempt to nudge you in the right direction. The best way to combat future errors is to keep it simple.

What is simple? Doing it in as few lines as possible, but still keeping it readable and easy to understand. Instead of 87 lines (304-217) of code, you can do it in 30 lines.

Also, it is a good mindset to lookout for places where the code could go into 'undefined' behavior. It would be good practice to include the size of the header in get_header(). If it is not available in get_header(), then there is no way to prevent potential buffer overflow/memory corruption.

#include <stdio.h> #include <stdlib.h> #include <string.h> int get_header(char *date, char *last_m, char *server, char *c_type, int c_len, char *header, int flag, int h_len); int main() { char header[500]; int n; n = get_header( "Fri, 09 Nov 2012 09:22:06 GMT", /* Date */ "Fri, 09 Nov 2012 09:10:32 GMT", /* Last modified */ "MySuperDuperServer/V 1.0.0.0", /* Server name */ "Text/html", /* Content type */ 1234, /* Content length */ header, /* Destination buf */ 1, /* 1 = write into header, 0 = Do not */ sizeof(header) /* header buf size */ ); printf("%s\n", header); exit(0); } /* Header lines should end with CR LF */ #define DATE "Date: %s\r\n" #define LAST_M "Last-Modified: %s\r\n" #define SERVER "Server : %s\r\n" #define C_TYPE "Content-Type: %s\r\n" #define C_LEN "Content-Length: %d\r\n\r\n" #define HEADER DATE LAST_M SERVER C_TYPE C_LEN int get_header(char *date, char *last_m, char *server, char *c_type, int c_len, char *header, int flag, int h_len) { char *d; int r; if (flag == 1) { d = header; } else if ((d = (char *)malloc(h_len)) == 0) { printf("out of memory\n"); exit(1); /* exit(EXIT_FAILURE); */ } r = snprintf(d, h_len, HEADER, date, last_m, server, c_type, c_len); if (flag == 0) { free(d); } return(r); }

I hope you find this helpful.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.