I have this code for an assignment where I want to create and open a file using the same name "in itself" of an imput file given via command line argument but with a different extention (example: I pass the file "filename.in" in terminal via argv and I want to create and open the file "filename.out"). However, I keep getting the error "Invalid size write of size 1" and a few others when I run my program in valgrind, and I really cannot see where they are coming from.
Code:
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include "roap.h"
int main(int argc, char** argv) {
FILE* filePtr;
LabList* head = NULL;
if (argc != 3) {
printf("Numero de argumentos errado! \n");
exit(EXIT_FAILURE);
}
char flag[] = "-s";
if ( strcmp(argv[1] , flag) != 0 )
{
fprintf(stdout, "Flag '-s' necessária para esta fase do projeto!"); //verifica se a flag -s está presente
exit(EXIT_FAILURE);
}
char *file_arg /*argumento indicado no terminal para referir ao ficheiro*/, *filename /*nome "próprio" do ficheiro*/, *file_arg_aux;
char dot = '.';
char ponto[] = ".";
char *extencao;
int read_ok;
file_arg = (char*) calloc(1, strlen(argv[2]) 1 ); //verifica se de facto a extensão é .in1
file_arg_aux = (char*) calloc(1, strlen(argv[2]) 1 );
strcpy(file_arg, argv[2]);
strcpy(file_arg_aux, argv[2]);
filename = strtok(file_arg, ponto);
extencao = strrchr(file_arg_aux, dot);
if ((read_ok = strcmp(extencao, ".in1")) != 0 )
{
fprintf(stdout, "Extensão inválida!");
exit(EXIT_FAILURE);
}
filePtr = fopen(argv[2], "r");
if (filePtr == NULL) {
printf("Erro ao abrir o ficheiro %s !\n", argv[2]);
exit(EXIT_FAILURE);
} else {
head = readLab(filePtr, head);
fclose(filePtr);
}
char extencao_out[] = ".sol1";
FILE* file_out = fopen( strcat(filename, extencao_out) , "w");
Valgrind output:
==123== Invalid write of size 1
==123== at 0x483EC5E: strcat (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==123== by 0x1095B2: main (main.c:60)
==123== Address 0x4a47051 is 0 bytes after a block of size 17 alloc'd
==123== at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==123== by 0x10944E: main (main.c:36)
==123==
==123== Syscall param openat(filename) points to unaddressable byte(s)
==123== at 0x4962D1B: open (open64.c:48)
==123== by 0x48E5195: _IO_file_open (fileops.c:189)
==123== by 0x48E5459: _IO_file_fopen@@GLIBC_2.2.5 (fileops.c:281)
==123== by 0x48D7B0D: __fopen_internal (iofopen.c:75)
==123== by 0x48D7B0D: fopen@@GLIBC_2.2.5 (iofopen.c:86)
==123== by 0x1095C1: main (main.c:60)
==123== Address 0x4a47051 is 0 bytes after a block of size 17 alloc'd
==123== at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==123== by 0x10944E: main (main.c:36)
Line 36: file_arg = (char*) calloc(1, strlen(argv[2]) 1 );
Line 60: FILE* file_out = fopen( strcat(filename, extencao_out) , "w");
CodePudding user response:
This ...
file_arg = (char*) calloc(1, strlen(argv[2]) 1 )
... allocates exactly enough space for a copy of argv[2]
, and assigns it to file_arg
. That's well and good, and it is afterward ok to strcpy(file_arg, argv[2])
.
Given the value of ponto
, this ...
filename = strtok(file_arg, ponto);
... truncates the string to which file_arg
points by overwriting the first '.'
(if any) with a string terminator, and it returns a copy of (pointer) file_arg
. That's ok in itself.
But then here:
FILE* file_out = fopen( strcat(filename, extencao_out) , "w");
, strcat(filename, extencao_out)
attempts to append the contents of string extencao_out
(".sol1"
) in place of the original extension, which you already verified, a bit awkwardly, was ".in1"
. Because exactly enough space was allocated for the original file name, no more, there is not enough room to accommodate the longer one that the program is now trying to construct. The allocated space is overwritten by one byte, just as Valgrind tells you.
I would suggest moving the declaration of extencao_out
far enough earlier that you can instead allocate like so:
file_arg = (char*) calloc(1, strlen(argv[2]) strlen(extencao_out) 1);
That will overallocate by four bytes with your present combination of extensions, but
- four bytes is negligible;
- it probably will place no extra memory burden at all on the system about 75% of the time; and
- it will be flexible towards other behavioral variations you might later want to support, such as input file names without extensions.
CodePudding user response:
Declare the variables on separate lines, with comments; each of these are pointers, and have declared/reserved no memory.
char* file_arg; /*argumento indicado no terminal para referir ao ficheiro*/
char* filename; /*nome "próprio" do ficheiro*/
char* file_arg_aux;
char* extencao;
Here you assign memory to file_arg, but only enough to hold the original argv string contents (plus null-terminator). Since you later reuse this buffer, you might just assign enough extra space for future needs.
file_arg = (char*) calloc(1, strlen(argv[2]) 1 ); //verifica se de facto a extensão é .in1
file_arg_aux = (char*) calloc(1, strlen(argv[2]) 1 );
Just use strdup() instead of calloc() and strcpy(),
strcpy(file_arg, argv[2]);
strcpy(file_arg_aux, argv[2]);
These functions both find the '.'/"." in the filename/file_arg, which you later use to append the new file extension (extencao?),
filename = strtok(file_arg, ponto);
extencao = strrchr(file_arg_aux, dot);
Here is your expected extension, which has length=4,
strcmp(extencao, ".in1")
Here is your new file extension, which has length=5,
char extencao_out[] = ".sol1";
Build your out filename with enough space to hold the filename and the new extension,
char* outfilename = calloc( strlen(filename) strlen(extencao_out) 1 );
strcpy(outfilename, filename);
strcat(outfilename, extencao_out);
FILE* file_out = fopen( outfilename, "w");