[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#796395: RFS: rolldice/1.14-1 ITA



Hi again Thomas,

>I've uploaded a new version to mentors fixing all the problems described
>above/below.


yes, mostly done I see just some issues left:

1)
> * Update debian/changelog to conform to DEP5


I guess you didn't mean changelog :p

2)
About the cmake file, please answer.

I really do not like custom makefiles, specially because they hide many troubles.

e.g. what if the user wants something like
export CC=clang in their rules file?

this just won't work :)

3) I just wrote a cmake file, and I even noticed you don't need math.h at all
(you seem to open dev/random for your random files)


4) you have some warning in the build log
gcc -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -g -c main.c
gcc -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -g -c rolldice.c
rolldice.c: In function 'get_num_sides':
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:183:43: note: declared here
int *get_num_sides(char *dice_string, int temp_int, int *res_int){
^
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:183:43: note: declared here
rolldice.c: In function 'get_num_drop':
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:200:42: note: declared here
int *get_num_drop(char *dice_string, int temp_int, int *res_int){
^
rolldice.c: In function 'get_num_rolls':
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:211:24: note: declared here
int *get_num_rolls(int temp_int, int *res_int){
^
rolldice.c: In function 'get_mutiplier':
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:221:43: note: declared here
int *get_mutiplier(char *dice_string, int temp_int, int *res_int){
^
rolldice.c: In function 'get_plus_modifier':
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:232:47: note: declared here
int *get_plus_modifier(char *dice_string, int temp_int, int *res_int){
^
rolldice.c: In function 'get_minus_modifier':
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
rolldice.c:243:48: note: declared here
int *get_minus_modifier(char *dice_string, int temp_int, int *res_int){
^
gcc -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security main.o rolldice.o -g -o rolldice -Wl,-z,relro -lm -lreadline


5) 
cat CMakeLists.txt 
cmake_minimum_required(VERSION 2.8)
add_executable(rolldice main.c rolldice.c)
install(TARGETS rolldice DESTINATION bin)
target_link_libraries(rolldice readline)


this is the cmake file I wrote for you.
Simple, works with clang

mkdir build
cd build
export VERBOSE=1
export CC=clang

it still needs some little tweaks:
a) find readline with cmake features (and fail if readline is not found)
b) generate the man page (trivial, but I don't want to do the work
if you don't want a cmake file)
c) install the manpage (lol)
d) think about the version.h, and maybe change it in version.h.in and generate it
from cmake
e) instead of using sed, use cmake features to configure a file (like manpage)

I can do all of them in a couple of minutes, but they are changes that might be good if
upstreamed :)

let me know :)

cheers,

G.


Reply to: