Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[doc] Revise howto #3222

Merged
merged 11 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions docs/examples/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
project_root_dir := $(strip $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST)))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To start a make process from other directory is allowed. This line prevents accidentally deletions in this case by clean target.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Makefile is really complex and it contains a lot of unnecessary things. This is just a test project, so please keep it as simple as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is not subject of the howto document. It only works and nice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the subject of this patch. Later if someone wants to modify this file it will be hard to understand it, because it is very complex. So I still recommend to simplify this. The whole Makefile can look like this:

CFLAGS = -Iincl

all:
	$(CC) $(CFLAGS) -c src/divide.c -o /dev/null
	$(CC) $(CFLAGS) -c src/main.c -o /dev/null

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is marked as resolved but it is not resolved yet. Please do not mark comment as resolved untill it is trully resolved or give me a reason why it is not resolved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @csordasmarton to simplify the makefile to the simple suggested one. At the end of the day it is only consists of 3 trivial files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • As you mentioned: "Later if someone wants to modify this file...". Why would somebody want to change the makefile? Because it changed the hierarchy of this "project". Your suggestion requires makefile modification too if some file is added or removed. Mine is resistant for that.

  • In the howto document sometimes we clean the project and re-make it. Clean feature is not supported by your suggested solution.

Our purpose with that howto is to inspire users to play with it. There are some other issues in these C sources. If the user tries to repair them then he should re-make the project. But your suggested solution will not make if the user changes the divide.h file.

  • The make can be called not only from that directory where the makefile resides. For example:

cd && make -f docs/examples/Makefile
should work. But your suggested makefile will not.

I think I have explained the requirements that were in my mind while creating that makefile.

I think if these requirements are valid then the "simplified" makefile will be at least comlex as mine is.

So, please specify your requirements against the makefile. And I will simplify it following them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With my solution there will be no generated files (output is redirected to /dev/null) so there is no need for clean target.

You don't have to step into the directory where the Makefile can be found, you can use the -C option of the make command for it if you would like to:

$ make -C docs/examples/
make: Entering directory '~/codechecker/docs/examples'
cc -Iincl -c src/divide.c -o /dev/null
cc -Iincl -c src/main.c -o /dev/null
make: Leaving directory '~/codechecker/docs/examples'

I still not recommend this complex Makefile for this simple example project but I will not block this PR. So on my side I finished the review of this patch.


runnable := $(project_root_dir)/ccexample

include_dir := $(project_root_dir)/incl
object_dir := $(project_root_dir)/objs
source_dir := $(project_root_dir)/src
sources := $(wildcard $(source_dir)/*.c)
objects := $(patsubst $(source_dir)/%.c,$(object_dir)/%.c.o,$(sources))
depends := $(patsubst $(source_dir)/%.c,$(object_dir)/%.c.d,$(sources))

CFLAGS = -MMD -MP -g -std=c99 -Wall
CFLAGS += -I $(include_dir)

all: $(runnable)

$(runnable): $(objects)
$(CC) $(LINKFLAGS) $^ -o $@

$(objects): $(object_dir)/%.c.o: $(source_dir)/%.c
@mkdir --parents $(object_dir)
$(CC) $(CFLAGS) $(CPPFLAGS) -c $< -o $@

.PHONY: clean
clean:
rm --recursive --force $(object_dir)
rm --force $(runnable)

ifneq ($(filter clean,$(MAKECMDGOALS)),clean)
-include $(depends)
endif
6 changes: 6 additions & 0 deletions docs/examples/incl/divide.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#ifndef DIVIDE_H
#define DIVIDE_H

long divide(long numerator, long denominator);

#endif // DIVIDE_H
6 changes: 6 additions & 0 deletions docs/examples/src/divide.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "divide.h"

long divide(long numerator, long denominator)
{
return numerator / denominator;
}
24 changes: 24 additions & 0 deletions docs/examples/src/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#include "divide.h"
zomen2 marked this conversation as resolved.
Show resolved Hide resolved

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


int main(int argc, char* argv[])
{
long result;
long test_case;

if(argc < 1) {
printf("Please specify testcase id.\n");
return 1;
}
if (strcmp(argv[1], "all")) {
test_case = strtol(argv[1], NULL, 10);
// Division by zero, only detected when ctu analysis is on
result = divide(test_case, 0);
}

return (int)result;
}
Binary file modified docs/images/static_html.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading