C -Element of a structure changes value after printf() call

Question

[Note that I am using C] I have an issue where when I call printf twice in a row I get "garbage data" the second time. The first printf will print out "rajan1" as expected, and the second printf will print out random symbols. I'm not sure if its something random or maybe an address. Does anyone know why I might be recieving these results? Here is a snippet from my code which highlights my problem (not the consecutive printf statements:

void main(void)
{
    struct listNode **pStart = (struct listNode**) malloc(sizeof(struct listNode**));
    load(pStart);
    printf(" %s\n",*(*pStart)->data->artist);
    printf(" %s\n",*(*pStart)->data->artist);
}

As I said the output is my expected string the first time(it prints "rajan1") and something random the second time(ussually symbols). I have scoured the internet for a similair problem but i haven't been able to find one. My searching indicates I may need to "free" a certain variable but I have no idea what I would need to free, or that possibly it is unique to my compiler version which I am unsure of(I am using whatever the default c compiler is in visual studio 2012 ultimate v11.0.61219.00 update 5). I have included all of my other code because hopefully someone can reproduce this. Most of all my question is under what circumstances can a printf statement change a value?

main.c

#include "Header.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void main(void)
{
    struct listNode **pStart = (struct listNode**) malloc(sizeof(struct listNode**));
    load(pStart);
    printf(" %s\n",*(*pStart)->data->artist);
    printf(" %s\n",*(*pStart)->data->artist);
}

header.c

#include "Header.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void load(struct listNode **temp)
{
    struct listNode **head = (struct listNode**) malloc(sizeof(struct listNode**));
    FILE *pF;
    char artist[100];
    char album[100];
    char song[100];
    char genre[100];
    int ntp = 0;
    int rating = 0;
    int minutes = 0;
    int seconds = 0;
    pF = fopen("Songs.DATA", "r");
    *temp = NULL;
    *head = (struct listNode*) malloc(sizeof(struct listNode*));
    while (fscanf(pF, "%100[^,],%100[^,],%100[^,],%100[^,],%2[^,],%2[^,],%5[^,],%2[^,]", &artist, &album, &song, &genre, &minutes, &seconds, &ntp, &rating) == 8)
    {
        *head = makeNode(makeRecord(artist, album, song, genre, makeSongLength(minutes, seconds), ntp, rating));
        insertNode1(temp,head);
        *temp=*head;
    }
    fclose(pF);
}

struct listNode * makeNode ( struct record * newData)
{
    struct listNode mem;
    struct listNode * pMem = &mem;
    pMem = (struct listNode *) malloc (sizeof (struct listNode*));
    pMem->data = (struct record *) malloc (sizeof (struct record*));
    pMem->data = newData;
    pMem->pNext = (struct listNode *) malloc (sizeof (struct listNode*));
    pMem->pNext = pMem;
    pMem->pLast = (struct listNode *) malloc (sizeof (struct listNode*));
    pMem->pLast = pMem;
    return pMem;
}

struct record * makeRecord(char*artist,char *album,char *song,char *genre,struct songLength *length, int ntp, int rating)
{
    struct record * pMem = NULL;

    pMem = (struct record *) malloc (sizeof (struct record));
    *pMem->artist = (char *) malloc ((sizeof (char) *100)+1);
    *pMem->artist = artist;
    *pMem->album = (char *) malloc ((sizeof (char) *100)+1);
    *pMem->album = album;
    *pMem->song = (char *) malloc ((sizeof (char) *100)+1);
    *pMem->song = song;
    *pMem->genre = (char *) malloc ((sizeof (char) *100)+1);
    *pMem->genre = genre;
    pMem->length = (struct songLength *) malloc (sizeof (struct songLength));
    pMem->length = length;
    pMem->ntp = (int) malloc (sizeof (int));
    pMem->ntp = ntp;
    pMem->rating = (int) malloc (sizeof (int));
    pMem->rating = rating;

    return pMem;
}

void insertNode1(struct listNode ** old, struct listNode ** fresh)
{
    if (*old == NULL)
        {
            (*fresh)->pLast=*fresh;
            (*fresh)->pNext=*fresh;
            return;
        }
    if ((*fresh)->data->artist <= (*old)->data->artist )
        {
            (*fresh)->pLast=(*old)->pLast;printf("1\n");
            (*fresh)->pNext=(*old);printf("2\n");
            if ((*fresh)->pLast==NULL)
            {
                (*fresh)->pLast==*old;
            }
            (*fresh)->pLast->pNext=*fresh;printf("3\n");
            (*fresh)->pNext->pLast=*fresh;printf("4\n");
        }
    else insertNode1(&(*old)->pNext,fresh);
    return;
}

struct songLength * makeSongLength(int minutes, int seconds)
{
    struct songLength * pMem = NULL;

    pMem = (struct songLength *) malloc (sizeof (struct songLength));

    pMem->minutes = minutes;
    pMem->seconds = seconds;

    return pMem;
}

header.h

#ifndef Header_h
#define Header_h


void sort();
void load(struct listNode **temp);
void display(struct listNode **head);
struct listNode * makeNode ( struct record * newData);
struct record * makeRecord(char*artist,char *album,char *song,char *genre,struct songLength *length, int ntp, int rating);
struct songLength * makeSongLength(int minutes, int seconds);
void insertNode1(struct listNode ** old, struct listNode ** fresh);

/*A record is a struct type which consists of the following attributes:
*      Artist – a string
*      Album title – a string
*      Song title – a string
*      Genre – a string
*      Song length – a struct type consisting of seconds and minutes, both integers
*      Number times played – an integer
*      Rating – an integer (1 – 5)*/
/*typedef struct record
{
    char *artist[100];
    char *album[100];
    char *song[100];
    char *genre[100];
    struct songLength *length;
    int ntp;
    int rating;
};

typedef struct songLength
{
    int minutes;
    int seconds;
};

typedef struct listNode
{
    struct record * data;
    struct listNode * pNext;
    struct listNode * pLast;
};

#endif

finally the text file which I am using to test this with(songs.DATA):

rajan1,summer india1,sweet summer1,pop1,21,31,01,11,
rajan2,summer india2,sweet summer2,pop2,22,32,02,12,

edit1: included the missing makeSongLength() at the bottom of header.c

It is because of Undefined Behavior. Haven't you been receiving warnings from the compiler telling you that something is wrong? `while (fscanf(pF, "%100[^,],%100[^,],%100[^,],%100[^,],%2[^,],%2[^,],%5[^,],%2[^,]", &artist, &album, &song, &genre, &minutes, &seconds, &ntp, &rating) == 8)` --> `while (fscanf(pF, " %99[^,],%99[^,],%99[^,],%99[^,],%d,%d,%d,%d,", artist, album, song, genre, &minutes, &seconds, &ntp, &rating) == 8)`. Also, you don't `free` the allocated memory. — Spikatrix, Sep 21 '15 at 05:56
If you have pointed-to objects changing after function calls that shouldn't affect them, that often means that you've returned a pointer to a stack-allocated value somewhere, instead of a heap-allocated value, but I didn't look very closely. Can you reduce this to a [Minimum Complete Verifiable Example](http://stackoverflow.com/help/mcve)? Doing so, you might solve your own problem, and if not, then it will be easier for others to help you if you give the minimum information to reproduce your problem. — zstewart, Sep 21 '15 at 05:58
makeNode(makeRecord(artist, album, song, genre, makeSongLength(minutes, seconds), ntp, rating)) : -> WHERE IS THIS makeSongLength(minutes, seconds) — asio_guy, Sep 21 '15 at 05:59
@zstewart There's tons of `foo->bar = malloc(...); foo->bar = &local_var;` — melpomene, Sep 21 '15 at 06:00
@Cool GuyYes there is a wraning saying that scanf is depricated. Does this mean I will continue to get these weird results unless I don't use scan f? — SparkleStep, Sep 21 '15 at 06:02
@SparkleStep No. And `scanf` isn't deprecated. It is just that Visual Studio encourages the user to use the "safe set of functions" like `scanf_s`. You should post a [mcve] in order to get better responses. — Spikatrix, Sep 21 '15 at 06:07
@Cool Guy Okay I will try to make a Minimal, Complete, and Verifiable example. Thanks for the link — SparkleStep, Sep 21 '15 at 06:10
@melpomene Well, that doesn't mean that there isn't some memory usage error somewhere (very likely when you get this kind of error), but ok yes, the usage of malloc means it is less likely to be a stack allocation problem. Regardless, as written, this question is unlikely to help future readers, and should probably be closed. — zstewart, Sep 21 '15 at 06:13
Avoid using Visual Studio to compile C code, it is not a conforming compiler. — Lundin, Sep 21 '15 at 06:18
@melpomene so if I am using foo->bar = &local_var; how can i keep that value without assigning the local_var to it? — SparkleStep, Sep 21 '15 at 06:25
@zstewart No, I meant to confirm your analysis! The `malloc` is a red herring (the pointer is overwritten, leaking memory), and your guess of pointers to local storage being returned is correct. — melpomene, Sep 21 '15 at 19:13

ameyCU · Accepted Answer · 2015-09-21T07:13:00.087

1

 while (fscanf(pF, "%100[^,],%100[^,],%100[^,],%100[^,],%2[^,],%2[^,],%5[^,],%2[^,]", &artist, &album, &song, &genre, &minutes, &seconds, &ntp, &rating) == 8)

This line will cause UB as in these array you don't left space for null character.Also format specifier to read int is %d. change to this -

while (fscanf(pF, " %99[^,],%99[^,],%99[^,],%99[^,],%d,%d,%d,%d,",artist, album,song,genre,&minutes, &seconds, &ntp, &rating) == 8)

void main(void) -> int main(void) or int main(int argc,char **argv)

don't cast result of malloc what you did in almost every case.

edited Sep 21 '15 at 07:13

answered Sep 21 '15 at 06:01

ameyCU

16,489
2
26
41

@ameyCU I've heard people say not to cast malloc in the past but it doesnt compile unless I do. Is that just a visual studio thing? – SparkleStep Sep 21 '15 at 06:09
@ameyCU Use `while (fscanf(pF, " %99[^,],%99[^,],%99[^,],%99[^,],%d,%d,%d,%d,", artist, album, song, genre, &minutes, &seconds, &ntp, &rating) == 8)`. Otherwise, the `fscanf` will fail in the next iteration on reading the second line because of the trailing `,` in the first line (See the contents of `Song.DATA`). – Spikatrix Sep 21 '15 at 06:12
@CoolGuy It is in data .Ohh Yep . – ameyCU Sep 21 '15 at 06:13
@SparkleStep What error do you get when you don't cast the result of `malloc`? – Spikatrix Sep 21 '15 at 06:13
@ameyCU Yes. It is better to add a space before the first `%99[^,]` so that the trailing newline character of the first line does not get read in the second iteration on reading the second line. – Spikatrix Sep 21 '15 at 06:14
@Cool Guy I says "error: a value of type "void*" cannot be used to initilize an entity of type [value type] – SparkleStep Sep 21 '15 at 06:20
@SparkleStep The SO community is obsessed by casting the result of malloc. It is not necessary to cast, so the cast is questionable. But it is hardly the end of the world if you do cast. [See this](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/22538350#22538350). In your case, you get errors either because VS doesn't realize it is C and tries to compile as C++, or possibly just because VS is a very poor C compiler. – Lundin Sep 21 '15 at 06:24
@SparkleStep I guess VS is compiling your code in C++ mode. – Spikatrix Sep 21 '15 at 06:28
1

He should use a c Compiler – Michi Sep 21 '15 at 07:50

C -Element of a structure changes value after printf() call

1 Answers1