Hi Again as mentioned in my last mail these are openbsd specific issues.
On Jan 23, 2008 4:49 PM, The Rasterman Carsten Haitzler <[EMAIL PROTECTED]> wrote: > On Wed, 23 Jan 2008 05:11:33 -0800 "laurent FANIS" <[EMAIL PROTECTED]> > babbled: > > > Hi, > > > > A long over due mail with the OpenBSD specific patches inlined. > > The files can be checked directly from > > http://www.openbsd.org/cgi-bin/cvsweb/ports/x11/e17/ > > This in the -current system (http://www.openbsd.org/faq/faq5.html#Flavors). > > This is based on a snapshot from 20071211. > > do you do the ports patching? if so you've done a great job at breaking e in > openbsd ports. > Not at all it works pretty well and i use it as my main desktop. > > In eet: > > > > --- src/lib/eet_lib.c.orig Mon Nov 12 11:14:04 2007 > > +++ src/lib/eet_lib.c Mon Nov 12 11:15:11 2007 > > @@ -497,6 +497,10 @@ eet_open(const char *file, Eet_File_Mode mode) > > else > > return NULL; > > } > > + else if (file_stat.st_size == 0) > > + { > > + return NULL; > > + } > > > > /* We found one */ > > if (ef && (file_stat.st_mtime != ef->mtime)) > > old - eet is even pickier. it requires the file be > 3 ints of size (which in > 99.9999% of cases int is 32bit - yes. technically it might not be - but if you > are on a system where it is not - 99.9999% of your software just broke > anyway :) ). not needed. > When i take a newer snapshot i will keep this in mind. > > > > Warnings: > > Four warnings:about "HAVE_STDLIB_H" being redefined. > > i don't know where this comes from, but config.h is included onle once in > Eet_private.h and that is included only once in each .c file. where are the > previous definitions? > > > Two warning about unsafe function : strcpy and sprintf instead of > > strlcpy and snprintf > > where? you might find that these are safe anyway as the pre-conditions make > them safe. if we missed them - telling us will be useful :). > > i can go through all of them: > > eet_lib.c: > check the code above the strcpy. the buffer is alloced to be bigger than the > string being strcpy'ed in. it's safe. > eet_data.c: > 1st instance: safe. malloced buffer of correct size exists to strcpy into. > 2nd instance: same as above > 3rd instance: same as above > 4th instance: commented out > 5th instance: commented out > 6th instance: commented out > > sprintf: buffers used are 64 bytes. this is more than large enough to hold any > possible output from _eet_double_to_string_convert() and the null terminator. > format is (up to a max): > > 0xX. (4 bytes - X is 1 hex value) > XXXXXXXXXXXXXXXX (16 bytes - hex) > pX (2 bytes) > XXXX (exponent in decimal max value with a max sized double will be 1023 - so > 4 > bytes) > > so a total buffer size requirement is 4 + 16 + 2 + 4 = 26 bytes, plus nul byte > terminator, so 27. we have 64. we're safe as houses. :) > I have no doubt about it but, it seems in some cases "safe" function are used in others "unsafe" function are used. I was merely mentioning the compilers warning. If all where safe function they wouldn't require passing over them later in development, again this is an opinion and i do not feel like fighting over it. We usually do not patch these in ports unless they are going to be fixed upstream, this is why i am only mentioning them. > > In Evas: > > > > $OpenBSD: patch-configure_in,v 1.1 2007/12/06 19:47:08 bernd Exp $ > > --- configure.in.orig Wed Dec 5 22:13:29 2007 > > +++ configure.in Wed Dec 5 22:13:42 2007 > > @@ -31,7 +31,7 @@ AC_SUBST(version_info) > > > > AC_FUNC_ALLOCA > > > > -MODULE_ARCH="$host_os-$host_cpu" > > +MODULE_ARCH="" > > AC_SUBST(MODULE_ARCH) > > AC_DEFINE_UNQUOTED(MODULE_ARCH, "$MODULE_ARCH", "Module architecture") > > you just broke multi-arch support if you install modules to ~/.evas/modules > (imagine your homedir is rsync'ed or nfs shared between a sparc solaris box, > an > x86 solairs box, a openbsd x86 box, an x86-64 linux box. each module will have > its binaries in a different arch dir - and will work - as long as you built it > for that arch+os. this patch summarily breaks that. > It is irrelevant in OpenBSD's case as EFL and E are installed via the packaging system, and that install the correct package in the correct place. Again i am not trying to push these patches. > > $OpenBSD: patch-src_lib_include_evas_common_h,v 1.1 2007/12/07 > > 09:49:58 mbalmer Exp $ > > --- src/lib/include/evas_common.h.orig Fri Dec 7 10:34:46 2007 > > +++ src/lib/include/evas_common.h Fri Dec 7 10:34:59 2007 > > @@ -46,6 +46,7 @@ > > #include <stdio.h> > > #include <stdlib.h> > > #include <unistd.h> > > +#include <stdint.h> > > #include <string.h> > > #include <signal.h> > > #include <setjmp.h> > > what problems did you fix by doing this? we rarely use uint32/64 etc. etc. > (and > friends). i'll put this in as it should be harmless unless your os is in a > retirement home awaiting its final days. > > > Warnings: > > evas_convert_rgb_8.c: In function > > `evas_common_convert_rgba_to_8bpp_rgb_221_dith': > > evas_convert_rgb_8.c:162: warning: right shift count is negative > > evas_convert_rgb_8.c: In function > > `evas_common_convert_rgba_to_8bpp_rgb_121_dith': > > evas_convert_rgb_8.c:195: warning: right shift count is negative > > evas_convert_rgb_8.c: In function > > `evas_common_convert_rgba_to_8bpp_rgb_111_dith': > > evas_convert_rgb_8.c:228: warning: right shift count is negative > > not important. limits it to 0 for us. that's the intended effect. > > > and: > > evas_outbuf.c: In function `evas_buffer_outbuf_buf_push_updated_region': > > evas_outbuf.c:268: warning: assignment from incompatible pointer type > > evas_outbuf.c:284: warning: assignment from incompatible pointer type > > evas_outbuf.c:310: warning: assignment from incompatible pointer type > > evas_outbuf.c:341: warning: assignment from incompatible pointer type > > unfortunately no directory for this file - so no way to know which one of > several evas_outbuf.c's it might be. chances are these are entirely harmless > and all we will do is shut the compiler up with some casts - which it does > anyway. > > > And: > > evas_engine.c: In function `eng_font_draw': > > evas_engine.c:855: warning: passing arg 3 of > > `evas_common_draw_context_font_ext_set' from incompatible pointer type > > evas_engine.c:855: warning: passing arg 4 of > > `evas_common_draw_context_font_ext_set' from incompatible pointer type > > evas_engine.c:855: warning: passing arg 5 of > > `evas_common_draw_context_font_ext_set' from incompatible pointer type > > evas_engine.c: In function `module_open': > > evas_engine.c:926: warning: assignment from incompatible pointer type > > same as above - i suspect this is xrender or gl engines - but i also know that > this is harmless and can be fixed with a cast - but it just makes the code > longer to shut the compiler up. > > > and: > > evas_engine_font.c: In function `_xre_font_surface_draw': > > evas_engine_font.c:178: warning: comparison is always false due to > > limited range of data type > > evas_engine_font.c:178: warning: comparison is always false due to > > limited range of data type > > harmless. we are using a common macro for rect clip/reduction and some values > are unsigned not signed, thus the comparison will be useless. the macro > handles > signed cases too and is generic, so this can be ignored. > > > and:a couple of warnings about "HAVE_STDLIB_H" being redefined in the > > jpg folder. > > jpg - libjpeg. not us. :) > > > In ecore: > > It seems ecore does not respect :--disable-ecore-evas-sdl > > --disable-ecore-sdl and still links to SDL if it exist, can someone > > please test this under linux. > > yes. a problem on our part - it links to libsdl - but doesnt use it. fixed. > Thank you > > Unsafe functions strcpy,sprintf and strcat instead of strlcpy,snprintf > > and strlcat > > > > In edje: > > edje_data.c: In function `_edje_edd_setup': > > edje_data.c:70: warning: assignment from incompatible pointer type > > edje_data.c:72: warning: assignment from incompatible pointer type > > edje_data.c:73: warning: assignment from incompatible pointer type > > edje_data.c:74: warning: assignment from incompatible pointer type > > edje_data.c:75: warning: assignment from incompatible pointer type > > edje_data.c:76: warning: assignment from incompatible pointer type > > edje_data.c:77: warning: assignment from incompatible pointer type > > edje_data.c:78: warning: assignment from incompatible pointer type > > all harmless. it's just bitching about const char * vs char * for generic > string return callback prototypes etc. > > > Unsafe functions:strcpy and strcat. > > as per eet. i'll make it shorter. all of them are ok. check the code. even > comments to the effect that the buffers are checked in the caller and are safe > - except one use case - alloca needed to alloc 1 more byte. there more funk > there than just looking at strcpy use though. > > > In embryo: > > unsafe functions: strcpy,sprintf and strcat. > > if it's embryo_cc - forget that. that is a pile of most ugly code inherited > from small/pawn and fixed to be 64bit clean and endianess-clean. i do not > desire to run through all its snprintf, strcpy etc. uses as it's only used as > a > stand-alone utility to compile embryo script by edje_cc into .edj files. if > someone has nothing to do with their weekends and evenings - they can take > this > on as a project :) > > as for the lib dir (libembryo) which is much more important as it is used > runtime and loads and runs embryo/small/pawn bytecode and is meant to be > entirely safe and sandboxed - no embryo script should ever bring down any > process using it. the strcpy/cat's are safe if you check the code itself and > the surrounding code that allocs or provides sufficiently large buffers. > > i fixed a simple path strcpy/cat thing in embryo_cc code in bin - but really. > that code is a jungle. > > > In E: > > --- configure.in.orig Wed Nov 7 11:45:47 2007 > > +++ configure.in Wed Dec 5 21:29:17 2007 > > @@ -70,7 +70,7 @@ AC_CHECK_HEADERS(security/pam_appl.h, [ > > AC_SUBST(VALGRIND_CFLAGS) > > AC_SUBST(VALGRIND_LIBS) > > > > -MODULE_ARCH="$host_os-$host_cpu" > > +MODULE_ARCH="" > > AC_SUBST(MODULE_ARCH) > > AC_DEFINE_UNQUOTED(MODULE_ARCH, "$MODULE_ARCH", "Module architecture") > > again - breaks multi-arch support for e like for evas. > > > @@ -148,6 +148,9 @@ AC_SUBST(ALL_LINGUAS) > > > > AM_GNU_GETTEXT([external]) > > AM_GNU_GETTEXT_VERSION([0.12.1]) > > +LIBICONV="$LTLIBICONV" > > +LIBINTL="$LTLIBINTL" > > +INTLLIBS="$LTLIBINTL" > > if test "x$LIBINTL" = "x"; then > > LIBINTL="$INTLLIBS" > > fi > > i am dubious about this - why? > This is only for the ports, IIRC this is to prevent linking to libintl.a and libiconv.a instead of the .so. > > --- src/bin/Makefile.am.orig Fri Nov 9 16:20:53 2007 > > +++ src/bin/Makefile.am Fri Nov 9 16:21:15 2007 > > @@ -345,7 +345,7 @@ e_xinerama.c > > > > enlightenment_init_LDFLAGS = @e_libs@ > > > > -setuid_root_mode = a=rx,u+xs > > +setuid_root_mode = a=rx,u+x > > install-exec-hook: > > @chmod $(setuid_root_mode) > > $(DESTDIR)$(bindir)/enlightenment_sys$(EXEEXT) || true > > you just broke sysactions. e can't shut down, reboot, suspend or hubernate > now. > it has to be setuid root to be able to run shutdown, reboot etc. as root. > there > is a sysactions.cfg file- text, that e installs that allows packagers to > define > access policies and what users, groups etc. can do what and what commands are > mapped to which sysaction. you probably should solve this with packaging and > install an appropriate sysactions.cfg into openbsd that still allows your > average desktop used to do these things as it is a necessary function of using > your pc (in almost all if not all cases - if you have access to the power > button - you should have access to these). the default sysactions.cfg is set > up > to "just work" as best as possible and works "as-is" on ubuntu - i guarantee. > distributions may/should customise it if needed. > As mentioned above the port system does not allow you to install suid stuff, If the user wants it he has to set it by hand. sysactions.cfg is being installed correctly via the port system. > > ##This is only to disaable the moduyles because they are not yet > > supported, work on this has been stalled. > > --- src/bin/e_config.c.orig Fri Dec 20 17:07:06 2007 > > +++ src/bin/e_config.c Fri Dec 20 17:07:48 2007 > > @@ -766,9 +766,9 @@ e_config_init(void) > > CFG_MODULE("ibox", 1, 0); > > CFG_MODULE("dropshadow", 1, 0); > > CFG_MODULE("clock", 1, 0); > > - CFG_MODULE("battery", 1, 0); > > - CFG_MODULE("cpufreq", 1, 0); > > - CFG_MODULE("temperature", 1, 0); > > + CFG_MODULE("battery", 0, 0); > > + CFG_MODULE("cpufreq", 0, 0); > > + CFG_MODULE("temperature", 0, 0); > > CFG_MODULE("pager", 1, 0); > > CFG_MODULE("exebuf", 1, 1); > > CFG_MODULE("winlist", 1, 1); > > not useful. these are needed for linux - and eventually the wizard will enable > or disable modules based on initial system probes the first time you run E. > i.e > - check if you have any battery (look for apm, acpi etc.) and if not - dont > load battery module. check for temp sensors - if you don't have any e knows > about - don't load temp module etc. > We will wait for the Wizard ;) till then i disabled them. > > --- src/bin/e_fm.c.orig Wed Nov 7 11:45:43 2007 > > +++ src/bin/e_fm.c Fri Dec 7 15:44:09 2007 > > @@ -104,7 +104,7 @@ struct _E_Fm2_Smart_Data > > E_Drop_Handler *drop_handler; > > E_Fm2_Icon *drop_icon; > > E_Fm2_Mount *mount; > > - char drop_after; > > + signed char drop_after; > > unsigned char drop_show : 1; > > unsigned char drop_in_show : 1; > > unsigned char drop_all : 1; > > why? char is normally signed. are you using a strange architecture? i can see > how this would break if char is not signed on a particular arch. > This is required for macppc. > > @@ -3214,7 +3214,7 @@ _e_fm2_uri_parse(const char *val) > > p = val + 7; > > if (*p != '/') > > { > > - for (i = 0; *p != '/' && *p != '\0' && i < > > _POSIX_HOST_NAME_MAX; p++, i++) > > + for (i = 0; *p != '/' && *p != '\0' && i < MAXHOSTNAMELEN; p++, i++) > > hostname[i] = *p; > > } > > hostname[i] = '\0'; > > ok - we are swapping one define for another. we are using the POSIX one. if > bsd > is not supplying posix defines... i'd suggest providing an autoconf check that > if _POSIX_HOST_NAME_MAX is not defined in system headers, to define it in > config.h > > > @@ -3247,9 +3247,9 @@ _e_fm2_uri_path_list_get(Evas_List *uri_list) > > { > > E_Fm2_Uri *uri; > > Evas_List *l, *path_list = NULL; > > - char current_hostname[_POSIX_HOST_NAME_MAX]; > > + char current_hostname[MAXHOSTNAMELEN]; > > > > - if (gethostname(current_hostname, _POSIX_HOST_NAME_MAX) == -1) > > + if (gethostname(current_hostname, MAXHOSTNAMELEN) == -1) > > current_hostname[0] = '\0'; > > > > for (l = uri_list; l; l = l->next) > > > > same here. > > > $OpenBSD: patch-src_bin_e_main_c,v 1.1 2007/12/04 17:43:15 bernd Exp $ > > > > The OpenBSD tar(1) doesn't have a -k option to preserve existing files. > > Use pax(1) with its -k option. > > and linux doesn't have pax. this breaks linux badly. also note - these "untar" > bits of code are just temporary during development. the wizard will take over > the job of populating user config files in the end. so i'm not concerned about > this. > So will wait for the wizard ;) . under OpenBSD tar -k fails and it breaks up badly so.... > > > --- src/bin/e_main.c.orig Mon Dec 3 13:59:16 2007 > > +++ src/bin/e_main.c Mon Dec 3 13:59:48 2007 > > @@ -1088,13 +1088,13 @@ _e_main_dirs_init(void) > > { > > snprintf(buf, sizeof(buf), > > "gzip -d -c < %s/data/other/desktop_files.tar.gz | " > > - "(cd %s/applications/ ; tar -xkf -)", > > + "(cd %s/applications/ ; /bin/pax -rk)", > > e_prefix_data_get(), > > efreet_data_home_get()); > > system(buf); > > snprintf(buf, sizeof(buf), > > "gzip -d -c < %s/data/other/desktop_order.tar.gz | " > > - "(cd %s/.e/e/ ; tar -xkf -)", > > + "(cd %s/.e/e/ ; /bin/pax -rk)", > > e_prefix_data_get(), > > homedir); > > system(buf); > > @@ -1105,7 +1105,7 @@ _e_main_dirs_init(void) > > { > > snprintf(buf, sizeof(buf), > > "gzip -d -c < %s/data/other/efm_favorites.tar.gz | " > > - "(cd %s/.e/e/ ; tar -xkf -)", > > + "(cd %s/.e/e/ ; /bin/pax -rk)", > > e_prefix_data_get(), > > homedir); > > system(buf); > > > > > > --- src/modules/cpufreq/Makefile.am.orig Fri Nov 9 15:16:55 2007 > > +++ src/modules/cpufreq/Makefile.am Fri Nov 9 15:17:26 2007 > > @@ -28,7 +28,7 @@ freqsetdir = $(pkgdir) > > freqset_DATA = \ > > freqset$(EXEEXT) > > > > -setuid_root_mode = a=rx,u+xs > > +setuid_root_mode = a=rx,u+x > > install-data-hook: > > @chmod $(setuid_root_mode) > > $(DESTDIR)$(freqsetdir)/freqset$(EXEEXT) || true > > breaks cpufreq! > Again this is OpenBSD specific.cpufreq is not working under OpenBSD and it is probably the module that would require the most work(probably a re-write for OpenBSD). > > Warnings: > > Alot of unsafe functions. > > details please. if it's anything like your list of unsafe str* sprintf* funcs > above - 90%+ are likely safe - if not all. just because a func is used does > not > make it unsafe. it is dependent on preconditions (eg the buffer is large > enough > for its work). i normally make fastidiously sure this *IS* the case. so > please > list in detail - and actually examine the code for the use cases (so i don't > have to keep doing it! :)). > Or u can use the "safe function" :P . All i did is report the nagging from the compiler, i haven't check in details all, but they seemed safe. > > e17 was tested under the following arch: > > i386, amd64,zaurus,macppc. > > > > Also many thanks to all the people that have helped > > (patches/testing/etc), too numerous to mention here but you know who > > you are. > > cool. thanks for this - but, you may want to really take note of why something > is there in the first place - most of these patches/notes just don't help. > they > break e or evas entirely, or break support, or break other os/architectures, > or > are simply reading out compiler warnings without actually checking whats > behind > them (trust me - we can read compiler warnings too :) vast likelihood is they > are still there because they have been looked at and then ignored henceforth > as they are safe). also remember that a use of strcpy/cat/sprints is by itself > not unsafe. its preconditions need to be met. chances are mostly that we did > that - thus i would say the onus is on those reporting to check the instance > of > that use in detail - then if it violates preconditions, report. :) > > Again i didn't post these trying to push anything, i was asked to post this on the mailing list. As mentioned above with these patches in the port system e17 work and is usable as a desktop. As soon as i get some free time i will post a complete build log. > -- > ------------- Codito, ergo sum - "I code, therefore I am" -------------- > The Rasterman (Carsten Haitzler) [EMAIL PROTECTED] > > Kind regards, Laurent. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
