1

My C program store a shell command output in a FILE, pass it to a char array and print it.

You don't need to look at the function in the program since the problem is on main.

The output of the command in my shell is:

$ cmus-remote -Q | grep 'tag title' | sed s/'tag      title'/''/g
 The View From The Afternoon

The char value in gdb is:

34              fclose(tmp2);
(gdb) print title
$4 = 0x603010 " The View From The Afternoon\n"
(gdb) n
51              char *lyric = lyrics_get(artist, title);
(gdb) print title
$5 = 0x603010 " The View From The After\001\002"

This is the code

#include <stdlib.h>
#include <stdio.h>
#include <curses.h>
#include "/usr/include/glyr/glyr.h"
#include <unistd.h>
#include <string.h>

char *lyrics_get(char *a, char *b);

int main(void)
{

    FILE *tmp1;
    FILE *tmp2;
    char *artist;
    char *title;
    tmp1 = popen("cmus-remote -Q | grep 'tag artist' | sed s/'tag artist'/''/g | sed '1s/^.//'", "r");
    tmp2 = popen("cmus-remote -Q | grep 'tag title' | sed s/'tag title'/''/g", "r");

    fseek(tmp1, 0, SEEK_END);
    size_t size1 = ftell(tmp1);
    rewind(tmp1);
    artist = malloc((size1 +1) * (sizeof(char)));
    fread(artist, sizeof(char), size1, tmp1);
    fclose(tmp1);

    fseek(tmp2, 0, SEEK_END);
    size_t size2 = ftell(tmp2);
    rewind(tmp2);
    title = malloc((size2 +1) * (sizeof(char)));
    fread(title, sizeof(char), size2, tmp2)
    fclose(tmp2);

    char *lyric = lyrics_get(artist, title);
    printf("%s", title);
    printf("%s", artist);
    printf("%s", lyric);
    return 0;

}

char *lyrics_get(char *a, char *b)
{
    glyr_init();
    atexit (glyr_cleanup);
    GlyrQuery q;
    glyr_query_init (&q);
    glyr_opt_type (&q,GLYR_GET_LYRICS);

    glyr_opt_artist (&q, (a));
    glyr_opt_title  (&q, (b));
    glyr_opt_force_utf8 (&q, true);
    GLYR_ERROR err;
    GlyrMemCache * head = glyr_get (&q,&err,NULL);
    return (head->data);
    glyr_free_list(head);
    glyr_query_destroy(&q);
}

Why it's different?

  • What does `cmus-remote -Q | grep 'tag title' | sed s/'tag title'/''/g | cat -A` print? – Keith Thompson Jul 29 '14 at 22:48
  • 4
    You can't do seeks on pipes. – Jonathan Leffler Jul 29 '14 at 22:49
  • 2
    Unrelated: In `lyrics_get`, your cleanup code doesn't run because it's after your return statement. – indiv Jul 29 '14 at 22:50
  • cmus-remote -Q | grep 'tag title' | sed s/'tagtitle'/''/g | cat -A prints: tag title The View From The Afternoon$ –  Jul 29 '14 at 22:55
  • This output doesn't match with the one in your question, which seems to indicate UB. Besides what Jonathan Leffler said, you don't 0-terminate your strings. – mafso Jul 29 '14 at 23:05
  • The output in this comments are with the cat -A argument wich isn't the one i use in the program. –  Jul 29 '14 at 23:12
  • But your debugger says the string ends with `"\1\2"` which doesn't match `"$"` from `cat -A`. So it seems, that the strings differ between different invocations of the program, which indicates UB. – mafso Jul 29 '14 at 23:16
  • 2
    Please, stop seeking on pipes and 0-terminate your strings. When this is fixed and you still have a problem, maybe update your question. Otherwise, delete it. – mafso Jul 29 '14 at 23:27
  • 1
    And error check your function calls...`malloc()` and `fread()` and `popen()` should all be error checked -- though `fread()` is the one most likely to cause trouble (the others will make the program crash if they fail; `fread()` won't necessarily do that). Have you printed the values of `size1` and `size2`? – Jonathan Leffler Jul 29 '14 at 23:31
  • But how i will pass the FILE to char without seeking? –  Jul 29 '14 at 23:31
  • `fread` returns the number of items read, so you have the size information you need to allocate memory. Just use the return values (which is also important for error checking). – mafso Jul 29 '14 at 23:40
  • 1
    I'd take a educated guess that the output from the commands is less than 4 KiB and use a buffer of that size. Check the return from `fread()` so you know how many bytes were read; if the number was 4096, then I'd consider allocating (more) memory and using that. Or I'd use POSIX `getline()` to read and allocate the memory. You should be able to run the command once, and have a single `sed` command delete the unwanted parts of output and munge the wanted parts appropriately. One thing to watch for would be the order of the tags, but you can handle that (`t:` and `a:` before the lines?) – Jonathan Leffler Jul 29 '14 at 23:44
  • I solved using another method, that don't make this error. http://stackoverflow.com/questions/25027819/pass-a-pipe-file-content-to-unknown-size-char-dynamic-allocated –  Jul 30 '14 at 18:45

0 Answers0