execl and wget command in a loop

Question

I have edited my previous question.

As I had got the problem and the changed the code, now I have a different problem. If I use execle command, it only downloads one image using the wget command, otherwise it prints all the image names on the screen if the wget command does not execute. I do not understand when there is a while loop, then why does it only print one image.

#include<stdlib.h>
#include<unistd.h>
#include<string.h>
#include<limits.h>
#include<fcntl.h>
#include<sys/stat.h>
#include<sys/types.h>
#include<stdio.h>

void main(int argc, char*argv[])
{
int iFlag;
char cline[100];
FILE*fil = fopen("index.html","rt");
if(fil==NULL)
{
    printf("Error in opening file");
}
char*tmpLine;
char*tok;
const char check[10] = "<img";
const char check2[10] = "src=";
char images[50];
strcpy(images,argv[1]);
while(fgets(cline,100,fil)!=NULL)
{
    if(strstr(cline,check)!=NULL)
    {
        tmpLine=strstr(cline,check);
        if(strstr(cline,check2)!=NULL)
        {
            tmpLine=strstr(cline,check2);
            tok = strtok(tmpLine,"\"");
            while(tok!=NULL)
            {
                tok = strtok(NULL,"\"");
                if(tok[0]!='/')
                {
                    strcat(images,"/");
                    strcat(images,tok);
                    printf("\nimage: %s\n",images);
                    iFlag = execle("/usr/bin/wget","wget","-o","logfile",images,NULL);
                    if(iFlag<0)
                      perror("EXECLE ERROR");
                    break;
                }
                else
                    break;
            }
            memset(&images[0], 50, sizeof(images));
            strcpy(images,argv[1]);
        }
    }

}

}

You should use a debugger. Due to buffering, `printf` are not reliable to spot position of segfault. — hivert, Mar 02 '14 at 09:05
If you want to write out debugging stuff, use `fprintf(stderr, ....)`, that isn't buffered and has a better chance of getting seen. — vonbrand, Mar 02 '14 at 10:39

score 8 · Answer 1 · answered Mar 02 '14 at 09:11

8

fil is probably NULL. Why?

FILE*fil = fopen("index.hmtl","rt");

Because you typo-ed the filename. This is why checking for errors is a good idea.

answered Mar 02 '14 at 09:11

user253751

57,427
7
48
90

2

Nice catch ... ;-) and another nice example why one shall **always** check the outcome of system calls. Human error is unpredictable ... – alk Mar 02 '14 at 09:15
It appears that I had made a mistake in specifying the file name and consequently, it was returning NULL. – talbinzal Mar 02 '14 at 09:18

score 4 · Answer 2 · edited May 23 '17 at 12:28

4

This line

        printf("%s\n",tok[0]);

does not make sense, as you pass a char where are char * is expected.

Either do

        printf("%s\n", tok);

or

        printf("%c\n", tok[0]);

Also this line

  char*images = (char*)malloc(100);

creates a memory leak, as the reference to the memory allocated to images is lost when leaving the context images is declared in without having free()ed it.

Also^2 :

In C there is no need to cast the result of malloc/calloc/realloc, nor is it recommended.

So the above line should just better be:

char * images = malloc(100);

Also^3: Always check the outcome of system calls, at least if the code relies on the values returned. Here: Check if fopen() failed, as on error NULL is returned, which will make the program choke if used as a valid file-pointer.

As a general advise on how to find a bug: Compile the code in question using symbols (option -g for gcc), then step through the "running" code using a debugger as gcc.

edited May 23 '17 at 12:28

Community

1
1

answered Mar 02 '14 at 09:06

alk

69,737
10
105
255

While correct, this is not the place where the OP is having problem for this question (probably next one would be about that...) – littleadv Mar 02 '14 at 09:09
What would be your suggestion for the usage of: char*images = (char*)malloc(100); because I need a char array in which I can put the url of the webpage and then concatenate the image name that I get from the file and download the image from that link. Every time, I only need to have the main url of the webpage so that I can concatenate the image name. – talbinzal Mar 02 '14 at 09:19
@user2507775: This stringly depends on wat you wnat to do with the content of `images`. – alk Mar 02 '14 at 09:25
@alk: I need to download those images after that. These images will be sent to a child process through a pipe and the child shall download them to a specified directory. – talbinzal Mar 02 '14 at 09:42

score 1 · Answer 3 · answered Mar 02 '14 at 09:14

you should check the return value of fopen(), as others already pointed it out.
printf("%s\n",tok[0]); should be printf("%s\n",tok);, if you want to print a string.
more serious,
```
    while(tok!=NULL)
    {
        strcat(images,tok);
    }
    tok = strtok(NULL,quote);
```
this tok = strtok(...); should be put inside that while loop, otherwise that tok will never be changed, and your program will crash finally.

execl and wget command in a loop

3 Answers3