Hi Emil, From: linux-media-owner@xxxxxxxxxxxxxxx [mailto:linux-media- owner@xxxxxxxxxxxxxxx] On Behalf Of Emil Velikov Sent: Wednesday, April 29, 2015 5:00 PM > > Hi Kamil, > > Allow me to put a few suggestions: > > On 29 April 2015 at 11:02, Kamil Debski <k.debski@xxxxxxxxxxx> wrote: > > This is the first version of the libGenCEC library. It was designed > to > > act as an interface between the generic CEC kernel API and userspace > > applications. It provides a simple interface for applications and an > > example application that can be used to test the CEC functionality. > > > > signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx> > > --- > > AUTHORS | 1 + > > INSTALL | 9 + > > LICENSE | 202 ++++++++++++++++ > > Makefile.am | 4 + > > README | 22 ++ > > configure.ac | 24 ++ > > doc/index.html | 345 +++++++++++++++++++++++++++ > > examples/Makefile.am | 4 + > > examples/cectest.c | 631 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/gencec.h | 255 ++++++++++++++++++++ > > src/Makefile.am | 4 + > > src/gencec.c | 445 +++++++++++++++++++++++++++++++++++ > > 12 files changed, 1946 insertions(+) > > create mode 100644 AUTHORS > > create mode 100644 INSTALL > > create mode 100644 LICENSE > > create mode 100644 Makefile.am > > create mode 100644 README > > create mode 100644 configure.ac > > create mode 100644 doc/index.html > > create mode 100644 examples/Makefile.am create mode 100644 > > examples/cectest.c create mode 100644 include/gencec.h create mode > > 100644 m4/.gitkeep create mode 100644 src/Makefile.am create mode > > 100644 src/gencec.c > > > > diff --git a/AUTHORS b/AUTHORS > > new file mode 100644 > > index 0000000..e4b7117 > > --- /dev/null > > +++ b/AUTHORS > > @@ -0,0 +1 @@ > > +Kamil Debski <k.debski@xxxxxxxxxxx> > > diff --git a/INSTALL b/INSTALL > > new file mode 100644 > > index 0000000..aac6101 > > --- /dev/null > > +++ b/INSTALL > > @@ -0,0 +1,9 @@ > > +To install libgencec run following commands: > > + > > +autoreconf -i > You might want add --force in here, otherwise the files will stay as-is > if you update libtool and friends "mid-flight". > Many projects include autogen.sh like the following. Feel free to add > it to libgencec. Thanks, I'll include this in the next version. > $cat autogen.sh > #! /bin/sh > > srcdir=`dirname "$0"` > test -z "$srcdir" && srcdir=. > > ORIGDIR=`pwd` > cd "$srcdir" > > autoreconf --force --verbose --install || exit 1 cd "$ORIGDIR" || exit > $? > > if test -z "$NOCONFIGURE"; then > "$srcdir"/configure "$@" > fi > > > > > --- /dev/null > > +++ b/configure.ac > > @@ -0,0 +1,24 @@ > > You can save yourself some headaches if you restrict old and/or buggy > autoconf versions which don't work with the project. > If I have to guess 2.60 should be ok. > AC_PREREQ([XXX]) Good suggestion, thanks. > > > +AC_INIT([libgencec], [0.1], [k.debski@xxxxxxxxxxx]) > > +AM_INIT_AUTOMAKE([-Wall -Werror foreign]) > > + > > +AC_PROG_CC > > +AM_PROG_AR > > +AC_CONFIG_MACRO_DIR([m4]) > > +AC_DEFINE([_GNU_SOURCE], [], [Use GNU extensions]) > > + > > Same for libtool - 2.2 perhaps ? > LT_PREREQ([XXX]) I agree. > > > +LT_INIT > > + > > +# Checks for typedefs, structures, and compiler characteristics. > > +AC_C_INLINE > > +AC_TYPE_SIZE_T > > +AC_TYPE_UINT16_T > > +AC_TYPE_UINT32_T > > +AC_TYPE_UINT8_T > > + > > +#AC_CHECK_LIB([c], [malloc]) > > +# Checks for library functions. > > +#AC_FUNC_MALLOC > > +l > > +AC_CONFIG_FILES([Makefile src/Makefile examples/Makefile]) > Would be nice if the library provides libgencec.pc file. This way any > users can avoid the explicit header/library check, amongst other useful > bits. Thanks for the suggestion, I'll look into this. > > --- /dev/null > > +++ b/examples/Makefile.am > > @@ -0,0 +1,4 @@ > > +bin_PROGRAMS = cectest > > +cectest_SOURCES = cectest.c > > +AM_CPPFLAGS=-I$(top_srcdir)/include/ > > +AM_LDFLAGS=-L../src/ -lgencec > The following seems more common (in the projects I've seen at least) > cectest_LDADD = $(top_builddir)/src/libgencec.la > > > diff --git a/m4/.gitkeep b/m4/.gitkeep new file mode 100644 index > > 0000000..e69de29 > Haven't seen any projects doing this. Curious what the benefits of > keeping and empty folder might be ? When I run autoreconf -i it complained about missing m4 folder, hence I added this filler file such that the folder is created. Any suggestion on how to do this better? > > > diff --git a/src/Makefile.am b/src/Makefile.am new file mode 100644 > > index 0000000..cb024f0 > > --- /dev/null > > +++ b/src/Makefile.am > > @@ -0,0 +1,4 @@ > > +lib_LTLIBRARIES = libgencec.la > > +libgencec_la_SOURCES = gencec.c > > +libgencec_la_LDFLAGS = -version-info 0:1:0 > You might want to add -no-undefined in order to prevent having a > library with unresolved symbols. > > Hope you find the above useful :-) Thank you so much for your review. It is my first real approach at autotools, so your comments are very much appreciated :) > > Cheers > Emil > -- Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
