how to create make in C

man system
man popen
man exec

Regards
Friedrich

There's also a portable function in standard C library, it's called system. Usage:
system("some command with command-line parameters");

The rules of how a makefile is processed are quite complex. Are you looking to fully support the correct semantics offered by make? This could be quite a lot of work getting the behavior right. GNU make is open source, have you considered looking at that?

You know the -f option of make can be used to specify the name of a make file, right? It doesn't' matter if it's called mymake you can still process it using make. You could, taking Gurudenis' example, simply do the following in your code...

system("make -f mymake");

well I think I am going to do these simple steps:

(1) First, memake looks for makefile then Makefile in the current
    directory.  If it can't find it, it exits with an error.
(2) Once memake finds the makefile, it goes through and parses the
    complete file.  In other words, it sequentially scans
    the files, building a "rules database", where it can lookup
    a rule by string name.
(3) Once the rules database is built, "memake" looks for a rule
    named "main".  If there isn't one, "memake" immediately
    exits.
(4) If there is a rule named "main", then "memake" gets the
    list of dependencies and list of build commands.
(5) Recursively, "memake" goes through each dependency and tries
    to "build" each one:  In other words, each single dependency
    becomes a target and the memake tries to build each one.
(6) Once all the dependencies have been built, then memake
    executes the list of build commands in order.
(7) Assuming nothing fails, memake exits.  If there was an error
    at any point, memake should have exited and aborted the process.

That's all what I am going to do

>> That's all

That's quite a bit I'd say ;)

yeah I know.. I am just really confused on how to do this..lol, it involves a lot of steps... on step 2 is this where I actually build a hash table to store the rules

>> on step 2 is this where I actually build a hash table to store the rules

If you want to use a hash table, then yes.

and how do I parse words by words? and so I am going to have a hash table of the rules as the key and the value as strings?

Are you sure you are ready for this kind of project ? Some advanced techniques are involved, and it's gonna take quite a bit of time to finish it (even for a very experienced programmer).

well yes, I think I am ready.. I need some help..  this is why I am ready.. it says this on the instruction:

It will respect a simplified format of current makefiles.  You
don't have to worry about handling default rules or macros within your memake.

I'm just wondering why you need to do this? Isn't this reinventing the wheel? Would it be worth clarifying as it might be a simpler solution can be offered.

>> I'm just wondering why you need to do this? Isn't this reinventing the wheel
Especially since, as I've already pointed out, you could just take the source code for GNU make and modify it -- assuming you can accept the restrictions of the GPL licence.

well this is an assignment I have to do.. I just learned C and I got this assignment..

>> I just learned C and I got this assignment..
Just learned C? Wow this is one hell of an assignment to be set then. Did you see what I8 said above?

"...it's gonna take quite a bit of time to finish it (even for a very experienced programmer)."

I agree with him!

I know it is... that's why I need help here.. is it going to be hard even with the simplification that I added above?

>> is it going to be hard even with the simplification that I added above?

Ok, off the top of my head...

(1) Relatively simple, there are API calls that you can use to do this (such as glob)

(2) Parsing the file out the file shouldn't be too hard, figuring out the dependencies (for example, are they files or other rules) will probably be a little trickier. I suspect (off the top of my head) that you'll need a tree like structure to store the dependencies and the rules associated with them, where the leaves of each node are its dependencies.

(3) Trivial

(4) This really depends on (2) and how well you designed structure of your rules database.

(5) This is dependent on (2) and (4)

(6) I'm not sure this one makes sense... once build issue build commands?

(7) Pretty trivial

I suspect the tricky part (so the part you should focus you design) will be (4)... coming up with a good structure to represent the rules and dependencies. If you do a good job of that it should just be a case of walking the tree and undertaking the actions.

Good luck.

-Rx.

>> (so the part you should focus you design) will be (4)...
I meant 2 not 4 :)

well yeah.. the instruction says to use a hash table for this, the key is the name of the rule, and the (dependencies, build-commands) is the value associated with the key. I already gave thought about this and this may be the most challenging part.. but then should I have a hash table of arrays?

If this is an assignment I presume you have a tutor? Have you thought about discussing this with him/her to (a) make sure you are going about this the right way and (b) to see if they can offer any tips. I presume they know what they are looking for in a solution? The fact the assignment specifies a specific data type suggests you are expected to demonstrate understanding of a specific principle.

I would start by writing a few example make files and then trying to sketch out on paper (ie, try to rationalize) some structures you could use to represent them. You'll probably find you can reject some structures pretty quickly as not being suitable. The rest are then candidates for implementation, go with what appears to be the simplest.

And I am confused on how to do recursion on hash tables?

and it is also more simplified by only having one target per line.. although in the original makefile we can have several target per line

How much time were you given for this project ?


>> And I am confused on how to do recursion on hash tables?

A hash table is a way to store data.
Recursion is a technique for structuring code.

They're not really linked. Your algorithm will say which data you need when (and you might make that algorithm recursive). And the hash table allows you to get the data you need.

1.5 weeks

I guess you'll have to cut down on the scope of the project if you want to finish it in that time ... Start coding I'd say ;)

yes, I know.. this is nuts.. I think my instructor is crazy giving me this kind of project... well if you are interested in looking at the specification.. it's right here:

http://rapidshare.com/files/106753111/spec.txt.html

if you don't mind reading it and post some suggestions here I would highly appreciate that and give you lots and lots of points

You know you can upload files on EE, don't you ? That would be a bit easier for us, plus the file would be kept with the question.


>> I think my instructor is crazy giving me this kind of project

I didn't say that. I said that you have to realize that it's gonna be a lot of work, and that you'll probably have to limit the scope.

Yeah I know you didn't say that, it's just me saying that he's crazy..haha..
spec.txt

please do help me read the specs and understand it, as I might interpret the specs in a wrong way and therefore creating an almost impossible project in 1 week or so

>> The object of this assignment is to familiarize yourself with
>> (a) recursion,
>> (b) executing UNIX commands from another program,
>> (c) system calls,
>> (d) using the file manipulation facilites of C,
>> (e) Makefiles,
>> (f) HashTables,
>> (g) dynamic memory allocation,
>> (h) more modules
>> (i) valgrind.

I assume that you used all (or most) of these in earlier projects ?


I'd say, just start with it ... The longer you talk about it, the more time you loose ;) Start with the parts that you know how to do. And then just build on that. When you encounter problems, we'll be glad to help you out.

that's actually my plan.. I've had recursion in java not in C, I don't know how to execute unix commands from another program, system calls I have no idea, file manipulation.. all I know is how to copy a file to another file and that's it.. I don't know how to search a file, etc

With all these skills I have, I don't even know where to start at.. I mean the first thing I have to do is to search if there's a makefile or Makefile.. I don't even know how to search a file..

If you think that I must be missing my class a lot, then that's wrong I never missed a single class

thanks Infinity you are my savior.. so did you read all the specs I posted?

diagonally yes.

and is it as hard as we think ?

and just for the info I am doing this in ansi C, not C++ or anything else

As I said : you should not waste time ;)

I don't under this part on the spec as well:

it's important that you turn in a Makefile that responds to
two commands: "make memake" and "make clean".

can you explain it more?

>> it's important that you turn in a Makefile that responds to

That's the Makefile for building the memake application. It's part of your project files.

oh okay, so just create a Makefile so that memake works..

Create a makefile to build the memkae application yes.

*memake*

okay cool.. so say that I were able to get the Makefile, and I get the first line of the file.. now I have to separate the target and dependencies right? how do I extract words from a line so I can put the target as a key in the hashtable

You can use strtok for example :

        http://www.cplusplus.com/reference/clibrary/cstring/strtok.html

>> I'd say, just start with it ... The longer you talk about it, the more time you loose
More haste makes more waste... by all means start coding the bits that are obvious but put a little planning into what's not otherwise you may end up doing some back-tracking, which would be wasteful. I still think the key to this is successfully planning your data structure, which you'll traverse to execute the rules. I think you should at least put a little thought into this before you start coding... or you don't know what you're coding towards!

>> I still think the key to this is successfully planning your data structure

A large part of the code does not depend (directly) on the data structure. And in any case, the assignment gave a pretty good idea of how to implement it :

        "The rules database should probably be implemented as a Hashtable where
        the key is the name of the rule, and the (dependencies, build-commands)
        is the value associated with the key."


>> I think you should at least put a little thought into this before you start coding...

Of course - that's not what I said ;) Allow me to quote what I said right after the phrase you quoted :

        "Start with the parts that you know how to do. And then just build on that."

>> And in any case, the assignment gave a pretty good idea
An idea is not a design. The statement uses the word "probably", meaning that's one way to go but not necessarily the way! It's a pretty high-level statement, which still requires some thought. For example. although the key may be obvious (according to this statement) what is the structure of the value going to be? This is a one to many relationship (potentially), so how will this be resolved? How will you know if you have a dependency or a command?

>> Start with the parts that you know how to do
Knowing how to do and knowing what to do are not the same thing.

Planning something properly is not lost time, conversely it often saves time. This is the only point I am really trying to make.

I am sure that I am going to use a HashTable for this.. I was thinking the values in the hash table could be an array of dependencies..

>> >> Start with the parts that you know how to do
>> Knowing how to do and knowing what to do are not the same thing.

Sure. But again, the assignment has a nice step-by-step plan of what should happen ... You can implement it one step at a time, and start with those steps you know how to do. Anyway ... Enough explaining ... Let's get to work heh ;)

yeah I am working on it now.. I have my hash table build.. but I am confused of what the values of the hash table is? array's of dependencies?

one more thing say we do ./memake Makefile then how do know for sure that the filr Makefile exists?

how do you check if a line starts with a tab?

okay quite a bit confused here...if I type in memake main, how do I find the makefile or Makefile of that main in the current directory? is there a specific command that would allow me to do that?

I still don't know what the value of the hash table would be stored at.. my hash table is build using chaining to handle collisions. Should I do this, say I have main : cstr.o hash.o main.o

I use main as the key and store cstr.o hash.o and main.o as the value associated with that key? so I have to build this recursively also, what is the base case for this then?

>> how do know for sure that the filr Makefile exists?
You could use the function access()
http://linux.die.net/man/2/access

>> how do you check if a line starts with a tab?
Just check that the first char of the string is '\t'

>> but I am confused of what the values of the hash table is? array's of dependencies?
You have to represent dependencies and commands in your hash value. This suggests your value will be two arrays or a single 2 dimensional array. You'll have a list of dependencies and a list of commands to execute once all the dependencies for the current rule/target have been met. Dependencies can be files or other rules/targets. I guess you need to look for the file and if that doesn't exist look in the hash table to see if it's a dependent rule/target and if it is then process it first. You'll need to remember the current rule so you can return to it once a dependent rule has been processed. One way to do this might be to implement a recursive algorithm.

a 2d array seems great...

yes I know that I must do this recursively, but what is the base case for this?

>> but what is the base case for this?
Eh?

well you mentioned that we have to do this recursively.. I am guessing that the base case here is when we have found main as the target?

processing it recursively, means that you create a recursive function that traverses the rules in the hash table, following the dependencies defined (and indeed starting with the main rule).

Do you know how to use recursive functions ? If not, take a look here :

        http://www.cplusplus.com/doc/tutorial/functions2.html

(scroll down to the paragraph titled "Recursivity")

>> I am guessing that the base case here is when we have found main as the target?
Ah ok, I see now :)

Normally, the 'all' target is the default rule in a make file. If this doesn't exist then it's the first rule/target at the top. Also, make allows the user to specify the required target on the command line. I don't think it'd be much of a task to implement the same convention as I've just described.

@evilrix : take a look at the assignment ;)

        "(3) Once the rules database is built, "memake" looks for a rule
            named "main".  If there isn't one, "memake" immediately
            exits."

>> take a look at the assignment
I wasn't saying to do it that way... I was just pointing out how make normally works and pointing out that to extend the behavior to this wouldn't be hard. Since this is explicit in the assignment I see little point in asking the question!

>> "(3) Once the rules database is built, "memake" looks for a rule...
Also, what you refer to is actually an example and not a statement of requirements within the spec. It does not state (that I could see) that the default target must be called main! It does however, state "it's important that you turn in a Makefile that responds to two commands: "make memake" and "make clean"". I'm sure this is not inconsistent with anything I have described above.

I was just pointing out why kinghalls was talking about the "main" rule ... I know it was just an example :)


btw, as far as I understand this :

        "it's important that you turn in a Makefile that responds to two commands: "make memake" and "make clean""

That just refers to the makefile for building the memake application, and not a makefile that should be used as input for memake. Do you interpret it the same way ?

>> Do you interpret it the same way ?
Yes, I re-read that and I agree with your interpretation.

Make files for building applications that process make files... this is now just getting too damn confusing :)

And all that on a sunday heh

lol..thanks guys for all that debate on sunday.. so after all the database is build I have to look for main.. in general.. what will I have to do next after I find main? find the dependencies associated in main?

I take a look at your link about recursion and yes, it has a base case and also a recursive function.. in general every recursive function has this.. that's what I mean.. what is this for my case?

There's two separate steps :

1) reading the makefile, and placing all rules in the data structure (don't care about dependencies for now)

2) find the rule that needs to be made, and get all its dependencies ... For each of those dependencies you call the function recursively

one more thing.. I still don't get how to store the rule and it's dependencies with the build command into the hash table.. the key for sure is the target, lets use an example for the sake of argument:

main: cstr.o hash.o main.o
         build commands here
cstr.o : cstr.c

so main is the key here and the value associated with main is cstr.o hash.o main.o

each of these dependencies is also a new target, cstr.o is a new target with cstr.c as the value.. right??

however how do I store the build commands in? I only have a hash table that handles collision by chaining (linked lists), do I have to modify it?

PS: could someone please answer my question on polyphase merge sort and external merge sort

For each rule, you need to store :

        (a) : the name of the target
        (b) : the dependencies (1 or more)
        (c) : the build command(s)

I'm sure you can come up with a nice way to store all of this data in a consistent way. Let's call it Rule.

The hash table is only used to find a Rule based on the target name.

oh I see.. so the hash table would be ideal to store a struct called Rule of all those things as values?? right?

so therefore I don't have to modify my current hash table implementation, instead just create a struct of Rule.. correct?

For example.

so there is another way to do this, and that's why you said for example?

PS: I need answers on this thread too if you don't mind:
https://www.experts-exchange.com/questions/23317626/polypahse-merge-sort-NEED-URGENT-HELP.html
https://www.experts-exchange.com/questions/23317620/Optimal-BST.html
https://www.experts-exchange.com/questions/23309812/improved-external-merge-sort.html

There's thousands of different ways of doing this. Yours is a good way, so I'd say go for it ;)

okay cool... Infinity08, why don't you help me to answer those other questions?

I don't have an infinite amount of time ;) I can't answer all questions heh

lol..but you keep answering this thread.. would you mind please looking at those threads I posted? nobody has responded to my thread

If I commit to a thread, then I'm in till the end ;) I might check them out later ...

okay..thank you so much! I hope you will because it's kind of important and I don't know where else to ask..

struct _list_t_ {
  char *commands;
  char *dependencies;
  char *target;
  struct _list_t_ *next;
};

so here's my struct.. it used to have only the key and the value in a node.. but now it has the target, which is the key and the list of dependencies is not what I am sure here.. there could be more then one dependencies in a target.. so what should I use?? an array?

Do you think a char* is the best way to store the dependencies ?

Think of it this way : will it be easy to split the string up when you need the separate dependency names ?


Btw, why did you make it a linked list ? I thought you wanted to use a hash table ?

yeah this is a hash table.. my hash table is an array of linked lists.. so that the hash functions maps to a specific index in an array, where it is the place to store the value..

I know a char* is wrong.. so I am thinking of using an array.. what do you think?

So, you're gonna make the nodes the Rule objects ... ok.


>> so I am thinking of using an array

Then only decide what kind of array (static or dynamic)

yes, that's what I am going to do ... each node in the linked list is a Rule.. I think it will be dynamic array as we don't know how many dependencies we're going to have right?

what about the build commands? I am guessing it's also an array as we don't know how many build commands are there

>> I think it will be dynamic array as we don't know how many dependencies we're going to have right?

Good idea ;)


>> what about the build commands? I am guessing it's also an array as we don't know how many build commands are there

If you can have more than one build command, then a dynamic array would indeed be a good idea :)

okay..I got it.. I've done all of that now.. and then what next.. say I have store all the target as the key and Rule as the value of the target??

oh and one more thing, each dependencies can also have other dependencies right, so in that case each dependencies that has other dependencies becomes another key?

As I said before : there's two separate stages :

1) reading the makefile, and placing all rules in the data structure (don't care about dependencies for now)

2) find the rule that needs to be made, and get all its dependencies ... For each of those dependencies you call the function recursively


The hash table just maps rule names to Rule objects.

The dependencies are resolved when you actually make a rule. And that happens recursively.

oh okay I got it.. so everytime I see a target in the makefile I just store it as a key and all the dependencies and build commands as the value.

so therefore I have to create a recursive function by starting at main?

It sounds like you got it yes :)

So, make sure you fill the hash table first, and then use the recursive function to resolve the dependencies and execute all the build commands.

okay.. so now the question comes to what is the recursive function here

first calling main.. main will look at it's dependencies and those dependencies will look on what it takes to build it and then when it's done it goes to the next dependencies, build it and return back again, and so on until all of the dependencies of main is build and then at last we could build main.. am I having this logic correctly?

one more thing, if I put the list of dependencies in an array then each elements in the array would be a string? correct? this also applies the same for build commands?

I think you've got it ... Just write down in code what you think should happen, and post it here ... It's a lot easier to read code than to read minds ;)

>> then each elements in the array would be a string? correct? this also applies the same for build commands?

Sounds good.

okay I will.. I still don't know how to put it in code though.. I have it in my mind

>> I still don't know how to put it in code though.. I have it in my mind

That's a programmer's job ;) Translate an idea to code :)

I am confused on the specification it says ince all the dependencies have been built then memake executes the list of build commands in order.. what does this actually means?

In the recursive function, you actually execute the build commands as you find them

yeah that's what I get, but the specs write it down as after I finish doing the recursion then I execute the build commands

Even if that's what was meant, that's not a good way of doing it. The reason of using a recursive function is to actually execute the build commands in order. If you don't do it in the recursive function, then you waste a very good opportunity.

>> I still don't know how to put it in code though.. I have it in my mind
You don't have to go straight to code. I often find it better to get all my thoughts out first so I can stop trying to think of them all at the same time and focus on smaller bits, hence I often realize complex designs using UML and then code from the resultant diagrams (in fact you can generate code from UML automatically using the right tools). Of course, you don't have to use UML, any thing that helps you realize your thoughts so you can convert them to code will do.

Anyway, I think I8 has this thread well in hand so in the interests of ensuring too many cooks don't spoil the broth I'll bid you good luck and depart this thread. I'll still keep an eye though in case you need help and I8 isn't around.

Regards,

-Rx.

thanks evilrix for the comments, okay so going back again to the first phase:

say in the makefile I have this:
main: cstr.0 hash.o main.o

I want main to be the key, but using string tokenizer it gives me main cstr.o hash.o main.o, how can I only get the main without all others?

I posted a little bit of code below..

char* pch = strtok(line, " :");
       while(pch != NULL){
         printf("%s\n", pch);
         pch = strtok(NULL, " :");
       }
      }

Select allOpen in new window

>> how can I only get the main without all others?

Only call strtok once - just to get the firs token :)

how do we free an array?

>> how do we free an array?

Depends how you allocated it ... For every malloc/calloc/realloc, you need a corresponding free.

what about the size of the hash table, what should it be?

>> what about the size of the hash table, what should it be?

Hash table dimensioning depends on how much you intend to put in the hash table. Just make sure that you can fit all data in it, and that it's still relatively easy to find the string you want (without too many collisions, or too large buckets).

and also I think I should have to insert methods in the hash table, one is to insert the dependencies and the other one for the build

I guess 50 would be a suitable size.. I guess there will be a problem when there's a collision using this method, as there will be one linked list to store the Rule and if there's a collision,my hash table will just put the Rule next to the corresponding linked list

What do you mean by insert methods in the hash table ?

Since you use a linked list as a bucket, a collision just makes the bucket grow ...

so I have to insert a key and a dependencies, and also a key with a build commands.. so therefore I need two insert functions in my hash table

or I could have a flag indicating what I am inserting into the hash table.. is it a dependencies or build commands

Didn't you put all data into one struct ? Why do you need two inserts then ?

>> is it a dependencies or build commands

There are always dependencies AND build commands for every rule (one of the two might be empty, but that doesn't matter).

okay here's what I've done.. please let me know if I did something wrong so I can change it.. suggestions could help too

int main(int argc, char *argv[]){
 
  if (argc == 1){
    fprintf(stderr, "memake: *** No targets specified and no makefile found. Stop.\n");
    return 2;
  }
  else{
    char* file_to_read = argv[1];
    FILE* fr = fopen(file_to_read, "r");
    if (fr == NULL){
      fprintf(stderr, "%s: trying to open file %s for read\n", strerror(errno), file_to_read);
      exit(3);
    }
 
    char* target = NULL;
    while(1){
      char* line = NULL;
      size_t line_length;
      int chars_read = getline(&line, &line_length, fr);
      if (chars_read == -1){
        if (feof(fr)){
          free(line);
          break;
        }
        perror("getline");
        exit(4);
      }
 
 
      if (line[0] == '\t'){
        printf("%s", line);
      }
      else{
       char* pch = strtok(line, ":");
       int h;
       for (h = 0; pch != NULL; h++){
         if (h == 0){
           target = pch;
         }
         else{
           printf("Target %s\n", target);
           printf("Dependencies %s\n", pch);
         }
         pch = strtok(NULL, " :");
       }
      }
 
      free(line);
    }
 
      fclose(fr);

Select allOpen in new window

>>       int chars_read = getline(&line, &line_length, fr);

You did not allocate memory for line ...


You're currently only printing - I assume that's temporary ?


>>          pch = strtok(NULL, " :");

":" as separator ? Are you sure ?

did I do this right?

struct _rule_t_{
  char** dependency;
  char** buildcmd;
};
 
typedef struct _rule_t_ rule_t;
 
/*a struct as a representation of a list*/
struct _list_t_ {
  struct rule  *rule;
  char *key;
  struct _list_t_ *next;
};

Select allOpen in new window

well yes that's only for temporary, the printing stuff.. I just want to make sure first.. if not as separator then as what?

>> did I do this right?

Why don't you put the rule name in the rule struct ?


>> if not as separator then as what?

I meant : is that the right separator ?

: is just to get the target... or should it be " :"?

what do you mean by the rule name ? the target?

so you mean like this?

struct _rule_t_{
  char** dependency;
  char** buildcmd;
  char* target;
};
 
typedef struct _rule_t_ rule_t;
 
/*a struct as a representation of a list*/
struct _list_t_ {
  rule_t  *rule;
  struct _list_t_ *next;
};
 
typedef struct _list_t_ list_t;

Select allOpen in new window

One more confusion I am having.. I must know the size of the array of dependencies and also the size of the array for the build commands.. how do I do this?? do I have to traverse the file twice?

>> : is just to get the target... or should it be " :"?

Yes, but you also use it to split up the dependencies - there's no ':' between those - just spaces.


>> so you mean like this?

Yes.


>> I must know the size of the array of dependencies and also the size of the array for the build commands.. how do I do this?? do I have to traverse the file twice?

You can add while you read, no ?

will this work?

#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>
#include "hash.h"
 
int main(int argc, char *argv[]){
 
  if (argc == 1){
    fprintf(stderr, "memake: *** No targets specified and no makefile found. Stop.\n");
    return 2;
  }
  else{
    HashTable* h = HashTableConstruct(50);
    char* file_to_read = argv[1];
    FILE* fr = fopen(file_to_read, "r");
    if (fr == NULL){
      fprintf(stderr, "%s: trying to open file %s for read\n", strerror(errno), file_to_read);
      exit(3);
    }
 
    rule_t rules = null;
    char* target = NULL;
    char** bld = NULL;
    char** dep = NULL;
    int bldIndex = 0;
    int depIndex = 0;
    while(1){
      char* line = NULL;
      size_t line_length;
      int chars_read = getline(&line, &line_length, fr);
      if (chars_read == -1){
	if (feof(fr)){
	  free(line);
	  break;
	}
	perror("getline");
	exit(4);
      }
      
      
      if (line[0] == '\t'){
	bld[bldIndex] = malloc(strlen(line) + 1);
	strcpy(bld[bldIndex], line);
	bldIndex++;
      }
      else{
       char* pch = strtok(line, ":");
       int h;
       for (h = 0; pch != NULL; h++){
	 if (h == 0){
	   target = pch;
	 }
	 else{
	   dep[depIndex] = malloc(strlen(pch) + 1);
	   strcpy(dep[depIndex], pch);
	   depIndex++;
	   printf("Target %s\n", target);
	   printf("Dependencies %s\n", pch);
	 }
	 pch = strtok(NULL, " :");
       }
      }
      
      bldIndex = 0;
      depIndex = 0;
      rules->dependency = dep;
      rules->buildcmd = bld;
      rules->target = target;
      HashTableInsert(h, target, rules);
      free(line);
    }
 
      fclose(fr);
 
 
  }
 
    return 0;
 
}

Select allOpen in new window

the thing that I am not sure with the code above is the array bld and also dep... I should first know what the size of this is right?

I know there's something wrong with this, can someone please tell me:

#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>
#include "hash.h"
 
int main(int argc, char *argv[]){
  
  if (argc == 1){
    fprintf(stderr, "memake: *** No targets specified and no makefile found. Stop.\n");
    return 2;
  }
  else{
    HashTable* t = HashTableConstruct(50);
    char* file_to_read = argv[1];
    FILE* fr = fopen(file_to_read, "r");
    if (fr == NULL){
      fprintf(stderr, "%s: trying to open file %s for read\n", strerror(errno), file_to_read);
      exit(3);
    }
 
    rule_t rules;
    char** bld = rules.dependency;
    char** dep = rules.buildcmd;
    int bldIndex = 0;
    int depIndex = 0;
    while(1){
      char* line = NULL;
      size_t line_length;
      int chars_read = getline(&line, &line_length, fr);
      if (chars_read == -1){
	if (feof(fr)){
	  free(line);
	  break;
	}
	perror("getline");
	exit(4);
     }
         
      if (line[0] != '\t'){
	  char* pch = strtok(line, ":");
      	  int h;
         for (h = 0; pch != NULL; h++){
	    if (h == 0){
	    printf("Target %s\n", pch);
	    HashTableInsert(t, pch, &rules); 
	    rules.target = pch;
	    bldIndex = 0;
	    depIndex = 0;
	 }
	 else{
	   dep = realloc(dep, sizeof(char *) * depIndex + 1);
	   dep[depIndex] = malloc(strlen(pch) + 1);
	   strcpy(dep[depIndex], pch);
	   rules.depSize = depIndex + 1;
	   depIndex++;
	   printf("Dependencies %s\n", pch);
	 }
	 pch = strtok(NULL, " ");
       }
      }
      else{
     	   bld = realloc(bld, sizeof(char *) * bldIndex + 1);
	   bld[bldIndex] = malloc(strlen(line) + 1);
	   strcpy(bld[bldIndex], line);
	   rules.bldSize = bldIndex + 1;
	   bldIndex++;
      }
      
      free(line);
    }
 
      fclose(fr);
 
 
  }
 
    return 0;
 
}

Select allOpen in new window

>>     rule_t rules = null;

is rule_t a pointer type ? What's null ?


>>       char* line = NULL;

You still haven't allocated memory for line ... See one of my earlier posts.


>> getline(&line, &line_length, fr);

What is getline ? Are you sure you need to pass &line and &line_length ? If you're not careful, you could have some nasty surprises overwriting memory that you're not supposed to.

Did you mean to use fgets ?

        http://www.cplusplus.com/reference/clibrary/cstdio/fgets.html


>>          for (h = 0; pch != NULL; h++){

This will pretty much loop forever. You need to split the input string up in the target (before the ':'), and its dependencies (after the ':', and separated by spaces). Then you read the next line(s) that contain the build commands. All this information goes into one rule struct. And then finally, at the end, you can insert it in the hash table.



I didn't look at the rest yet, because you need to fix the above problems first ...

rule_t is not a pointer type, here it is:

struct _rule_t_{
  int depSize;
  int bldSize;
  char** dependency;
  char** buildcmd;
  char* target;
};

typedef struct _rule_t_ rule_t;

why can't I use getline?? it just extract a line from a file??

>> rule_t is not a pointer type, here it is:

My point is that you were apparently using it as if it were a pointer ;)


>> why can't I use getline?? it just extract a line from a file??

Did you write a getline function ? There's no such function in standard C. How is it defined ?

I am using one that is used by the GNU_SOURCE_

in which part am I interpreting it as a pointer? so what should I do to initialize rule_t first?

look at this:

http://linux.die.net/man/3/getline

and I don't get why my loop won't stop? I tried it and it stopped

>> in which part am I interpreting it as a pointer?

What does this mean ?

>>     rule_t rules = null;

What's null ?


>> so what should I do to initialize rule_t first?

You can do :

        rule_t rules = { 0 };


>> I am using one that is used by the GNU_SOURCE_

Ok. It's not standard though ... I'd use fgets, but it's your choice.


>> and I don't get why my loop won't stop? I tried it and it stopped

My mistake. It's your strange indentation that confused me. You should properly align the {}'s to avoid confusions like this.

okay I posted my new code here, still doesn't work.. can you help me:

#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>
#include "hash.h"
 
int main(int argc, char *argv[]){
  
  if (argc == 1){
    fprintf(stderr, "memake: *** No targets specified and no makefile found. Stop.\n");
    return 2;
  }
  else{
    HashTable* t = HashTableConstruct(50);
    char* file_to_read = argv[1];
    FILE* fr = fopen(file_to_read, "r");
    if (fr == NULL){
      fprintf(stderr, "%s: trying to open file %s for read\n", strerror(errno), file_to_read);
      exit(3);
    }
 
 
    rule_t* rule = malloc(sizeof(rule_t));
    char** bld = NULL;
    char** dep = NULL;
    int bldIndex = 0;
    int depIndex = 0;
    int flag = 0;
 
    while(1){
      char* line = NULL;
      size_t line_length;
      int chars_read = getline(&line, &line_length, fr);
      if (chars_read == -1){
	if (feof(fr)){
	  free(line);
	}
	perror("getline");
	exit(4);
      }
 
      if (line[0] != '\t'){
	if (flag == 1){
	  rule->dependency = dep;
	  rule->buildcmd = bld;
	  rule->bldSize = bldIndex + 1;
	  rule->depSize = depIndex + 1;
	  rule_t* ru = malloc(sizeof(rule_t));
	  RuleConstruct(rule, ru);
	  HashTableInsert(t, rule->target, ru);
	  RuleDestruct(rule);
	  flag = 0;
	  bldIndex = 0;
	  depIndex = 0;
	}
	 char* pch = strtok(line, ":");
      	 int h;
         for (h = 0; pch != NULL; h++){
	    if (h == 0){
	     rule->target = pch;
             printf("Target %s\n", pch); 
	    }
	    else{
	      dep = realloc(dep, sizeof(char *) * (depIndex + 1));
	      dep[depIndex] = malloc(strlen(pch) + 1);
	      strcpy(dep[depIndex], pch);
	      printf("Dependencies %s\n", dep[depIndex] );
	      depIndex++;
	    }
	   pch = strtok(NULL, " ");
         }
	   
      }
      else if (line[0] == '\t'){
	 flag = 1;
	 bld = realloc(bld, sizeof(char *) * (bldIndex + 1));
	 bld[bldIndex] = malloc(strlen(line) + 1);
	 strcpy(bld[bldIndex], line);
	 printf("%s\n", bld[bldIndex]);
	 bldIndex++;
      }
      free(line);
    }
      fclose(fr);
 
      
  }
 
    return 0;
 
}

Select allOpen in new window

>> still doesn't work

Define "still doesn't work".

it creates a segmentation fault whenever I call RuleConstruct, here's the code for RuleConstruct:

rule_t* RuleConstruct(rule_t* rule, rule_t* newrule)
{
  char** dep = (char**) malloc(sizeof(char*) * rule->depSize);
  int h;
  for (h = 0; h < rule->depSize; h++){
     dep[h] = malloc((strlen(rule->dependency[h]) + 1) * sizeof(char));
     strcpy(dep[h], rule->dependency[h]);
  } 
 
  char** bld = (char**) malloc(sizeof(char*) * rule->bldSize);
  int i;
  for (i = 0; i < rule->bldSize; i++){
     dep[i] = malloc((strlen(rule->buildcmd[i]) + 1) * sizeof(char));
     strcpy(dep[i], rule->buildcmd[i]);
  }
 
  char* target = (char*) malloc((strlen(rule->target) + 1) * sizeof(char));
  strcpy(target, rule->target);
   
  newrule->dependency = dep;
  newrule->buildcmd = bld;
  newrule->bldSize = rule->bldSize;
  newrule->depSize = rule->depSize;
  newrule->target = target;
 
  return newrule;
}

Select allOpen in new window

>> it creates a segmentation fault

At which line ? Check whether rule and newrule are valid pointers. Also check rule->dependency, rule->buildcmd and rule->target.

Btw, why are you copying everything ?

don't I have to copy rule to a new struct so that I can use it to store the next rule??

So I decided to pull off the RuleConstruct and just do an insertion HashTableInsert, and here's the code.. the part where it fails is when it tries to call StringHash

boolean HashTableInsert(HashTable *this, const char* key, rule_t* rule)
{
  list_t *temp;
 
  int_u4 hashval = StringHash(key) % this->size;
 
  /* Does item already exist? */
  for (temp = this->table[hashval]; temp != NULL; temp = temp->next){
    if (strcmp(key, temp->rule->target) == 0){
       RuleDestruct(temp->rule);
       rule_t* newrule;
       temp->rule = RuleConstruct(rule, newrule);       
       return false;
    }
  }
  list_t *new_list;
  if ((new_list = malloc(sizeof(list_t))) == NULL)
    outOfMemory_(__LINE__, __FILE__, "struct node creation");
  rule_t* newrule = malloc(sizeof(rule_t));
  new_list->rule = RuleConstruct(rule, newrule);
  new_list->next = this->table[hashval];
  this->table[hashval] = new_list;
  this->length = this->length + 1;
  return true;
}

Select allOpen in new window

>> don't I have to copy rule to a new struct so that I can use it to store the next rule??

Why not immediately put it in the new struct then ?


>> the part where it fails is when it tries to call StringHash

How does it fail ? Is key a valid pointer to a string ? What is the implementation of StringHash ?

okay now I got it all set.. I just have memory leaks everywhere.. could you help me to fix those?

#define _GNU_SOURCE
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <stdlib.h>
#include "hash.h"

int main(int argc, char *argv[]){
 
  if (argc == 1){
    fprintf(stderr, "memake: *** No targets specified and no makefile found. Stop.\n");
    return 2;
  }
  else{
    HashTable* t = HashTableConstruct(50);
    char* file_to_read = argv[1];
    FILE* fr = fopen(file_to_read, "r");
    if (fr == NULL){
      fprintf(stderr, "%s: trying to open file %s for read\n", strerror(errno), file_to_read);
      exit(3);
    }


    rule_t* rule = malloc(sizeof(rule_t));
    char** bld = NULL;
    char** dep = NULL;
    char* target = NULL;
    int bldIndex = 0;
    int depIndex = 0;
    int flag = 0;

    while(1){
      char* line = NULL;
      size_t line_length;
      int chars_read = getline(&line, &line_length, fr);
      if (chars_read == -1){
      if (feof(fr)){
        free(line);
      }
      perror("getline");
      exit(4);
      }

      if (line[0] != '\t'){
      if (flag == 1){
        rule->dependency = dep;
        rule->buildcmd = bld;
        rule->bldSize = bldIndex;
        rule->depSize = depIndex;
        rule->target = target;
        HashTableInsert(t, target, rule);
        RuleDestruct(rule);
        rule = malloc(sizeof(rule_t));
        dep = NULL;
        bld = NULL;
        target = NULL;
        flag = 0;
        bldIndex = 0;
        depIndex = 0;
      }
       char* pch = strtok(line, ":");
             int h;
         for (h = 0; pch != NULL; h++){
          if (h == 0){
           target = StringConstruct(pch);
             printf("Target %s\n", pch);
          }
          else{
            dep = realloc(dep, sizeof(char *) * (depIndex + 1));
            dep[depIndex] = malloc(strlen(pch) + 1);
            strcpy(dep[depIndex], pch);
            printf("Dependencies %s\n", dep[depIndex] );
            depIndex++;
          }
         pch = strtok(NULL, " ");
         }
        
      }
      else if (line[0] == '\t'){
       flag = 1;
       bld = realloc(bld, sizeof(char *) * (bldIndex + 1));
       bld[bldIndex] = malloc(strlen(line) + 1);
       strcpy(bld[bldIndex], line);
       printf("%s\n", bld[bldIndex]);
       bldIndex++;
      }
      free(line);
    }
      fclose(fr);

     
  }

    return 0;

}

>> I just have memory leaks everywhere.. could you help me to fix those?

You just have to make sure that for every malloc/calloc/realloc there's a corresponding free.

can you see any from the code above?  okay so now I already build a hash table of dependencies and with it's target.. now I can move on to the next step to do this recursively

>> can you see any from the code above?

Any malloc/calloc/realloc that doesn't have a corresponding free is a memory leak. The first one that I see is :

    rule_t* rule = malloc(sizeof(rule_t));

okay I already did a RuleDestruct at the end of this while loop..

>> okay I already did a RuleDestruct at the end of this while loop..

That's not very consistent ... Why create a RuleDestruct, but use malloc directly in main ?

you mean I have to do a RuleConstruct too? but I don't have any rule to construct at the beginning, that's why I am just doing a malloc in the first time

Do what you like ;) I'm just saying it's better to be consistent.

haha..okay.. I still can't see any more leaks, but using valgrind I found 19 blocks are not freed..

Valgrind should also tell you where the leaks are, no ?

Wow this threads grown since I last checked it :)

>>  still can't see any more leaks, but using valgrind I found 19 blocks are not freed..
If you'd like help interpreting your Valgrind output post it here.

okay I freed them all, I forgot to free the hash table after I am done with it.. so here comes the fun part where I have no clue how to code.. recursively start from main to build commands... so from main I go to access it's dependencies until the dependencies found it's build cmd and then I build it.. and this is where I am confused after I am done I should return back.. how do I return? I need a base case here

>> how do I return? I need a base case here

What do you mean by a base case ? The only thing the recursive function has to do is call the function recursively for each dependency of the current rule, and then execute the build command(s) for the current rule.

so I don't need a base case?? call the function recursively until I found a target that has only 1 or 0 dependencies then I execute the build command?

Did you understand what I explained ? I'll repeat it :

        "The only thing the recursive function has to do is call the function recursively for each dependency of the current rule, and then execute the build command(s) for the current rule."

Read it carefully.

okay..so it will continue to call it's dependency until a target has only one dependencies.. then we execute the build command

If you just code the function like I explained, the dependencies will be handled "automatically" by the recursive calls.

so I am going to have to create a function outside of main to do this then.. so that it can call itself??  so whenever it calls it's dependency then it will execute it's build commands? what if it has another dependency?

>> so I am going to have to create a function outside of main to do this then

Yes.

For the rest. I'll repeat once more what I said earlier :

        "The only thing the recursive function has to do is call the function recursively for each dependency of the current rule, and then execute the build command(s) for the current rule."

Read it very carefully, and let every part sink in ... If there's a part you don't understand, then ask about it.

the first time it calls, so what do I pass to the recursive function? the next dependency?

The function takes as argument a rule object or the name of a rule. Whatever you like best.

The first time you call it, you obviously pass the main rule.

what I don't get is this, each dependency becomes a target right?

what if the last target does not have any dependency or build commands?

so the first time I call it, it takes the main object instead of the main dependencies?

>> what if the last target does not have any dependency or build commands?

It usually will have at least a build command - otherwise there's no reason to make a rule.

But if neither are present, you can just return :)

does the method return anything? it just returns void and takes a rule as a parameter right?

>> it just returns void and takes a rule as a parameter right?

That should be sufficient. Unless you want to return an error status in order to break off the build process if an error is encountered. But you can add that later. For now you can keep it void.

so for example:

hot.o : hot.c
         build command here

hot.o will call hot.c recursively, but when it tries to search hot.c, it's not in the hash table... if it's not in the hash table then it does not have any build commands..

Off to bed now. See you tomorrow :)

>> if it's not in the hash table then it does not have any build commands..

You need to check whether the file exists. If it does, then it doesn't need build commands, because the file is already there. If the file doesn't exist, and there's no rule to create it, then that's an error in the makefile.

what do you mean by the file is already in there?

>> what do you mean by the file is already in there?

That it exists. Do you understand the point of a rule in a makefile ? Its intent is to create the target by executing the build commands associated to it (including its dependencies).

yes.. I do understand.. because I am storing the dependencies in an array then I have to sort of use a counter to stop calling the next dependencies? I know I have to do it recursively but I have to stop somewhere in that recursive function

>> but I have to stop somewhere in that recursive function

Obviously - and that happens at the end of the recursive function, when it returns.

the function just returns a void right? so I just do a return?

I am still confused with this recursive thing.. as far as I know in recursion there's always what we call a base case and a recursive function, the function that calls it self.. but you mention there's no base case here as it's only calling it recursively.. if there's no base case then how is the function going to stop

>> the function just returns a void right? so I just do a return?

Didn't we already discuss that ?

        https://www.experts-exchange.com/questions/23314228/how-to-create-make-in-C.html?anchorAnswerId=21354714#a21354714


>> but you mention there's no base case here as it's only calling it recursively..

I didn't say anything like that. I don't even know what you mean by a base case.

Do you mean what the function will be doing apart from the recursive calls ? If so - that's the build commands. And if you check back in my description of how you should implement the recursive function, that's right there :

        https://www.experts-exchange.com/questions/23314228/how-to-create-make-in-C.html?anchorAnswerId=21354592#a21354592

what I mean by a base case here is:

say I want to count factorial recursively, the base case is:

if (number <= 1)
  return 1

See my previous post ...

>>The only thing the recursive function has to do is call the function recursively for each dependency of the >>current rule, and then execute the build command(s) for the current rule.

I'll try my best to interpret this

void recursive(rule_t* r){
  int index = 0;
  if (r->depSize == index)
     return;
  else{ 
  recursive(rule->dependency[index])
  execute build command here
  index++;
  }
}

Select allOpen in new window

No. You need to call the function recursively for EACH dependency - ie. you need a loop for that.

And then you execute the build command(s).

What's the index for ?

so I need to use a for loop for each dependency?

>> so I need to use a for loop for each dependency?

You need to have a loop that recursively calls the function for each dependency.

so inside the loop is a recursive function? or is it the recursion which is the loop?

You call the function recursively inside the loop. The loop will make sure that it gets called more than once - ie. once for each dependency.

yes so this is what I am saying.. we have a loop for the list of dependencies.. starting on the dependencies of main.. right?? then inside the loop the recursive function will call the dependency as a rule and so on?

am I getting this right?

I'm not sure, because the way you explain it is not very clear. As I said before : just try to put it in code - that's a lot easier to read than to read minds.

well yeah.. this is what I've been trying to do, put it in code.. however I always have think of a base case when I heard the term recursion...

what I mean't before is that we start by traversing the dependency list of main and then for each one of those we do a recursive call until all build command of that dependecy(which becomes a rule) is executed

Just read back the comments I've made earlier about the "base case". Just put in code what I explained.

I read your comments about twenty times, but still I am expecting a base case for every recursive function.. and you didn't give me any clue about any base case or what so ever...

you just said to call it recursively and it will return by it self.. which makes no sense to me as every recursive function has a base case.. in the factorial example that I posted above the base case for this function is when n<=1  

>> but still I am expecting a base case for every recursive function..

And as I said earlier : the base case as you call it is if there are no dependencies for the rules, and you just have to execute the build commands for the rule.

        https://www.experts-exchange.com/questions/23314228/how-to-create-make-in-C.html?anchorAnswerId=21358487#a21358487

well okay then I think I might have a problem with parsing the strings now:

main : cstr.o hash.o main.o

I can parse main fine.. but after I got main as the target it also counts the : as a dependencies.. how do eliminate these?

 char* pch = strtok(line, " ");
      	 int h;
         for (h = 0; pch != NULL; h++){
	    if (h == 0){
	     target = StringConstruct(pch);
             pch = strtok(line, ":");
             printf("Target %s\n", target); 
	    }
	    else{
	      dep = realloc(dep, sizeof(char *) * (depIndex + 1));
	      dep[depIndex] = malloc(strlen(pch) + 1);
	      strcpy(dep[depIndex], pch);
              printf("Length is %d\n", strlen(dep[depIndex]));
	      printf("Dependencies %s\n", dep[depIndex] );
	      depIndex++;
	    }
	   pch = strtok(NULL, " ");
         }

Select allOpen in new window

ignore my post above..

okay I happen to do the recursive steps correctly and I can execute the build commands recursively.. now comes the small details.. I think I have to look for a particular time stamp of the file right? if it's different then I have to build a new one? am I right? is this how make works?

>> and I can execute the build commands recursively..

The build commands should not be executed recursively ... Just execute them at the end of the recursive function.


>> I think I have to look for a particular time stamp of the file right? if it's different then I have to build a new one? am I right? is this how make works?

if none of the dependencies have a more recent timestamp than the target, then the build commands for that target don't have to be played.

yeah I mean that's what I mean't.. I execute them after the recursion is finished and I can run the main well.. so that's a sign it works..

>>if none of the dependencies have a more recent timestamp than the target, then the build commands for >>that target don't have to be played.

so using this example

cstr.o : cstr.c
      build command here

if cstr.c does not have a more recent timestamp than the cstr.o then we don't build it again? right?

and this is also assuming that cstr.c exists in the directory..if not the we build it?

>> and this is also assuming that cstr.c exists in the directory..if not the we build it?

You always execute the dependencies first. So, since you executed the dependencies, the file cstr.c should exist (unless an error occurred). If it doesn't, then treat that as an error. If it does exist, then you check the timestamp of the file.

oh so you mean later on when we want to execute the build command at main where it uses cstr.c to build main it checks the timestamp?

Yes. But not just for the main rule ... For every rule.

so for every case where it builds something that has a cstr.c it tries to find the timestamp of cstr.c first and compare it to what? the current time? if it's the same then we don't need to build cstr.c again?

Let me repeat what I said earlier :

        "if none of the dependencies have a more recent timestamp than the target, then the build commands for that target don't have to be played."

just to make it clear eveytime we see a dependency and a target we compare both first then see if they are equal then we do nothing..

in the example below, we compare the timestamp cstr.o with cstr.c if cstr.c hash more recent timestamp then we build the commands?

cstr.o : cstr.c

That's not what I said. I never said first (I said between executing the dependencies and executing the build commands), and I never said equal (I said later than).

between executing the dependencies and executing the build commands, so in other words just before executing the build commands right?

oh yes and I don't mean to say equal, I mean if the dependencies time stamp  are later than the target time stamp, my bad

If one of the dependencies has a later timestamp than the target, the build commands have to be executed.

hmm.. I seem not to find a file called cstr.o : cstr.c in my directory.. I can find cstr.c because I can see it in my directory but not cstr.o..

maybe I don't quite understand how make workds

cstr.o : cstr.c
     build command

so the build command here is what's needed to have cstr.o right? if before building the build command we try to find cstr.o then it doesn't exist as it hasn't been build yet.. if that's so then how are we comparing the timestamps for a file that hasn't been build yet?

A file that doesn't exist has no filestamp ... what do you think should happen ?

then it should build the build command as it does not exist, when later on cstr.o is used as another dependency then we could compare?

If you execute make twice in a row, then the cstr.o might also exist the second time.

>>If you execute make twice in a row, then the cstr.o might also exist the second time.

what do you mean by this? so the purpose of doing the timestamp is so that we don't create cstr.o twice the second time right?

my only confusion is only this, you mentioned that I should compare the timestamp between the target and the dependencies.. well how can we get the timestamp of the target as it hasn't been build yet and that's what actually needs to be build from the dependencies..

unless we execute the build commands of cstr.o first, we won't have the file cstr.o

unless.. make has been executed once and it's been executed again, then we have cstr.o somewhere from the previous make.. is this what you're trying to say?

The purpose is so that we don't do unnecessary work. If cstr.o is up-to-date, then there's no need in re-generating it. It saves precious time building a project.

The purpose is so that we don't do unnecessary work. If cstr.o is up-to-date, then there's no need in re-generating it. It saves precious time building a project.

if cstr.o itself doesn't exist at the beginning then we build it right?

>> well how can we get the timestamp of the target as it hasn't been build yet and that's what actually needs to be build from the dependencies..

It seems I have to repeat everything. Refer back to this post :

         https://www.experts-exchange.com/questions/23314228/how-to-create-make-in-C.html?anchorAnswerId=21370147#a21370147

and your response to it.

I response it already, but you did not mention whether my response was correct or not

I thought it was pretty obvious :) And since I didn't say it was wrong in the very next post ...

well okay then, then I'll build as you mentioned..

is there any particular thing that I should do after I finished with the timestamps?

Test your code :)

You can also post it here for me to have a look at.

okay.. I'll do a test on my code

oh I just had another question.. in a make file usually there's a target called clean with no dependency.. how do I get to this recursively as this is not one of the dependencies that "main" has

What happens if you do :

        make clean

you think ?

The main target is only started when you do :

        make main

hmm.. am I suppose to do:

make makefile

or

make main

I think it would be reasonable to do make something here then I will look for the associated makefile for that main or clean

       make main

will build the main target from makefile.

if that's so the second argument after make blah blah blah

is the starting target? am I right?

so if do make main, then first make will look for the makefile or Makefile file first

If you check the assignment, you'll see that that's exactly what happens.

okay I did..

So, is it working ?

I'll let you know back tomorrow.. I'll post again if I have questions

I just had a question.. after we compare the timestamp between the target and dependencies.. what if the the target is NOT older than the dependencies? what do we do? nothing? as we have the file in the system but it hasn't been modified?

so if I put:

main.o : main.c
        echo "testing"

after I execute make clean.. testing will be printed, but if I execute it again.. it doesn't print testing? am I interpreting this correctly?

>> what if the the target is NOT older than the dependencies?

As I said : then there's no need to re-create the target, so there's no need to execute the build commands for it.


>> but if I execute it again.. it doesn't print testing?

I don't see any build command that would create a main.o file ...

let's just assume for the sake of argument that echo "testing" could build main.o

one more thing say I have this rule of main:

main : big.o bob.o ban.o
       some build command here
       some build command here

so every time we want to execute main build command we have to check for big.o bob.o ban.o if the time stamp of one of them is newer.. if one of them happens to be newer.. do we build it again?

okay I think I need some help, I've been trying to work this out all night long.. and can't see where I am doing wrong here.. can you help me why this is not working?

it's not working because I think I might have done the recursion wrong...

suggestions where I did it wrong

static void processRule(rule_t* r, HashTable* hash)
{
  if (r == NULL || r->depSize == 0){
    return;
  }
  else{
    int h;
    rule_t* temp;
    for (h = 0; h < r->depSize; h++){
      temp = HashTableLookup(hash, r->dependency[h]);
      processRule(temp, hash);
    }
    printf("BUILD TARGET IS %s WITH BUILD SIZE %d\n", r->target, r->bldSize);
  }
  int i;
  for (i = 0; i< r->depSize; i++){
    if (FileExists(r->dependency[i]) == 0){
	  if (HashTableContains(hash, r->dependency[i]) == 1){
	    printf("HAHAHA\n");
	    processRule(HashTableLookup(hash, r->dependency[i]), hash);
	  }
	  else{
	    printf("ERROR\n");
	    exit(4);  
	  }
    }
    else{
   
      if (FileExists(r->target) == 1){
	if (TimeInfo(r->target, r->dependency[i]) == 0){
        int h;
	 printf("FILE %s HAS BEEN EDITED, NEED TO REBUILD\n", r->dependency[i]);
	 for (h = 0; h< r->bldSize; h++){
	   system(r->buildcmd[h]);
	 }
	else{
	 printf("FILE %s DOESN'T NEED TO BE BUILD AS NOTHING IS MODIFIED\n", r->dependency[i]);
	}
      } else{
	printf("%s DOES NOT EXIST, SO BUILD ONE\n", r->target);
	int m;
	for (m = 0; m< r->bldSize; m++){
	   system(r->buildcmd[m]);
	 }
      }
    }
   }
  }
}
 
	 
	  
static boolean FileExists(const char* filename)
{
  struct stat buf;
  struct stat* bp = &buf;
  int e = stat(filename, bp);
  if (e == 0){
    if (S_ISREG(buf.st_mode)){
      return true;
    }else{
      return false;
    }
  }else{
    return false;
  }
}
 
static boolean TimeInfo(char* file1, char* file2)
{ 
  if (FileModTime(file1) > FileModTime(file2)){
    return true;
  }
  else{
    return false;
  }
}
 
static time_t FileModTime(const char* filename)
{
  if (FileExists(filename)){
   struct stat buf;
   int e = stat(filename, &buf);
   if (e ==1)
     return 0;
   printf("Time for %s is %d\n", filename, buf.st_mtime);
   return buf.st_mtime;
  }
  printf("FILE NOT FOUND");
  return 0;
}

Select allOpen in new window

>> if one of them happens to be newer.. do we build it again?

Yes.


>>   if (r == NULL || r->depSize == 0){

Do you want to support rules without dependencies (but with build commands) ?


>>           processRule(HashTableLookup(hash, r->dependency[i]), hash);

Why do you do this a second time ? You already did that.

If one of the dependency files doesn't exist at this point, then it means an error occurred.


>>       if (TimeInfo(r->target, r->dependency[i]) == 0){

I assume that TimeInfo does its work correctly ?



>>        for (h = 0; h< r->bldSize; h++){
>>         system(r->buildcmd[h]);
>>       }

This code is in your function twice. In order to avoid problems when modifying your code, it's better to only have one instance of the code that does the same thing. You can easily re-arrange the if's to achieve this. Also, the use of return's in the correct location will help you.

>>Do you want to support rules without dependencies (but with build commands) ?

yes, this is the case for clean, isn't it?


>>Why do you do this a second time ? You already did that.

>>If one of the dependency files doesn't exist at this point, then it means an error occurred.

gotcha!

>>I assume that TimeInfo does its work correctly ?
is it not working correcly? where? which part?

>>Also, the use of return's in the correct location will help you.

  there's only one return in the base case, is this wrong?

>> yes, this is the case for clean, isn't it?

Indeed. The point of my comment was that you're currently NOT supporting that due to the quoted piece of code.


>> >>I assume that TimeInfo does its work correctly ?
>> is it not working correcly? where? which part?

I don't know if it works correctly ... I assume it does. You'll need to verify that.


>> >>Also, the use of return's in the correct location will help you.
>> 
>>   there's only one return in the base case, is this wrong?

That comment was to help you get rid of the redundant code. Using returns in the correct location will help you do that.

what do you mean by redundant code here? the code after the recursive step?

and yes I do believe the TimeInfo works

>> what do you mean by redundant code here? the code after the recursive step?

Refer to my previous post :

>> >>        for (h = 0; h< r->bldSize; h++){
>> >>         system(r->buildcmd[h]);
>> >>       }
>> 
>> This code is in your function twice. In order to avoid problems when modifying your
>> code, it's better to only have one instance of the code that does the same thing.
>> You can easily re-arrange the if's to achieve this. Also, the use of return's in the
>> correct location will help you.

so I should put another return function somewhere? or should I put that redundant code inside the recursive function?

Right now you have something like :

if (condition1) {
    if (condition2) {
        /*do_something*/                   /* <--- */
    }
    else {
        /*do_something_else*/
    }
}
else {
    /*do_something*/                       /* <--- */
}

Notice that the two indicated lines are the same ? That's duplicated code. How can you re-organize this code to have the /*do_something*/ only once ?

I am still confused of what you mean by putting the return at the right spot.. can you be more specific about this? clues? hints would help

Ignore that for now - just think about the question "How can you re-organize this code to have the /*do_something*/ only once ?", and try to answer it.

is there a possible way to rearrange this? because one is if a condition and the other one is if not the condition..

I guess this will work:

if ((FileExists(r->target) == 0 || (FileExists(r->target) == 1 && TimeInfo(r->target, r->dependency[i]) == 0)){
 printf("FILE %s HAS BEEN EDITED, NEED TO REBUILD OR FILE %s DOES NOT EXIST\n", r->dependency[i], r->target);
 int h;
 for (h = 0; h< r->bldSize; h++){
   system(r->buildcmd[h]);
 }
}
else{
 printf("FILE %s DOESN'T NEED TO BE BUILD AS NOTHING IS MODIFIED\n", r->dependency[i]);
}

Select allOpen in new window

That's one way, yes :)

okay..now with the return statement.. what's that all about? can you give me some clues / hints? or even more guidance

>> okay..now with the return statement.. what's that all about? can you give me some clues / hints? or even more guidance

Doesn't matter ... You did it without using return's, and that's fine.