-
Notifications
You must be signed in to change notification settings - Fork 384
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
+510
−246
Merged
[doc] Revise howto #3222
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
ce810da
Improve howto
zomen2 b06dfaa
Step 2
zomen2 6a64207
[doc] Revise howto
zomen2 fd64ba8
Step 4
zomen2 5cef3e2
[Doc] Revise howto
zomen2 4be94bf
[Doc] Revise howto
zomen2 ca540d1
[Doc] Revise howto
zomen2 6644738
[Doc] Revise howto
zomen2 df0d62c
[Doc] Revise howto
zomen2 4010446
[Doc] Revise howto
zomen2 0a0dd8a
[Doc] Revise howto
zomen2 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
project_root_dir := $(strip $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))) | ||
|
||
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to simplify this Makefile based on this Makefile: https://github.com/Ericsson/codechecker/blob/master/web/tests/projects/cpp/Makefile
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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.
There was a problem hiding this comment.
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: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.