-3

I am trying to concatenate two strings in C and receive a "Thread 1: signal SIGABRT" error.

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

int main() {
    char name[50];
    ifile = fopen("stats.list", "r");

for(;;) {
    fscanf(ifile, "%s%f%f", name, &sky, &stddev);
    if (feof(ifile))
    break;

    char ext[5] = ".par";
    dataparsFile = strcat(name, ext);

    dataparsFile = fopen(dataparsFile, "w");

    fprintf(dataparsFile, "%s\n",
            "stuff gets read in to file named after new string";

    fprintf(ofile, "phot ");
    fprintf(ofile, "%s%s%s%s%s%s \n",
            ", datapars=", dataparsFile);
}

fclose(ifile);
fclose(ofile);

The goal of the code is to take an image name that is read in and add on the .par extension. Then, I want to open a file with that name of image+.par and write into it. Since I will have a couple hundred such files, I need to loop through them with the name changing each time.

  • 1
    are you sure it's not on the `ofile = fopen("strcat(name,suffix)", "w");` line? because it is buggy for sure there! And are you sure your `name` buffer is big enough? 30 seems meagre. can you show us the output of your program (since there are well-placed `puts` statements) – Jean-François Fabre Aug 31 '16 at 20:56
  • Ouch. You should probably read about buffer overflows. Ouch. – jcaron Aug 31 '16 at 21:20
  • Also, you're supposed to provide a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve) illustrating your issue. Yours is definitely not minimal. – jcaron Aug 31 '16 at 21:22
  • Note that your use of `feof()` is far too late; test the call to `fscanf()` directly. See [`while (!feof(file))` is always wrong](http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) for discussion of a variant of what you're doing, but what you're doing is also incorrect for basically the same reasons. – Jonathan Leffler Aug 31 '16 at 21:32
  • Note that the `ofile = fopen("strcat(name,suffix)", "w");` line opens a file with a fixed name and parentheses as part of the name. Not precisely an invalid name, but nothing like what you intended. —— The code changed; it now has `ofile = fopen("datapars", "w");`. You need to open the file whose name is in the variable `datapars`, not the file named `"datapars"`. No quotes around the variable name. That was the problem before, too, with the longer version. And test your file pointer after `fopen()` before using it; you will crash if you don't. – Jonathan Leffler Aug 31 '16 at 21:35
  • so I should just remove the quotation marks around it? Ok, and it will use the concatenated name – Michelle Gurevich Aug 31 '16 at 21:37
  • Thank you for the link on feof(). I will look into this – Michelle Gurevich Aug 31 '16 at 21:37
  • Yes, just remove the quotes — it will use the value of the variable. – Jonathan Leffler Aug 31 '16 at 21:38
  • Please read up on how to create an MCVE ([MCVE]) — aka SSCCE ([Short, Self-Contained, Correct Example](http://sscce.org/)). What you had before was better than what I see now. I know it's hard at first to gauge what's appropriate. – Jonathan Leffler Aug 31 '16 at 21:40
  • you have heavily edited your question, still a lot of problems remain. My answer is now obsolete. provide a [mcve] and a lot of people will help. – Jean-François Fabre Aug 31 '16 at 21:48
  • `fprintf(dataparsFile,` --> `fprintf(ofile,` – BLUEPIXY Aug 31 '16 at 21:59
  • @BLUEPIXY I have a different file in the program called ofile and can't name this one the same. – Michelle Gurevich Aug 31 '16 at 22:01
  • The first argument of `fprintf` must be `FILE*`. `dataparsFile = strcat(name, ext); dataparsFile = fopen(dataparsFile, "w");` : They are inconsistent. – BLUEPIXY Aug 31 '16 at 22:10

1 Answers1

0

The problem is name is not initialized. You see, in strings use a convention, they are any sequence of ASCII (probably some other printable characters, but in principle just ASCII) that must be followed by a '\0' byte that marks the end of the string.

Your name array doesn't have this '\0' so strcat() tries to find it but it fails and perhaps it reads beyond the end of the array, although anyway reading uninitialized data is undefined behavior.

The way strcat(dst, src) works is pretty much like this

char *
strcat(char *const dst, char *src)
{
    // Make a pointer to keep dst's address
    // unchanged and return it
    char *ptr = dst;
    // Compute search for the end of the destination
    // string to start copying there
    while (*ptr != '\0')
        ptr++;
    // Copy all the characters from `src' until the '\0'
    // occurs
    while (*src != '\0')
        *(ptr++) = *(src++);
    *ptr = '\0';
    return dst;
}

As you see, this is very inefficient if you call strcat() many times, and it will certainly not work if you pass either of the parameters before initializing it.

In fact, it's terribly unsafe because there is no bound checking, the caller has to make sure that the destination array is large enough to hold both strings.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97