[parted-devel] porting to FreeBSD

[Jim Meyering]

"Siavosh Benabbas" <sbenabas at gmail.com> wrote:

> Hi,
> Thanks for replying.
>
> On 2/28/07, Jim Meyering <jim at meyering.net> wrote:
>> Thanks for taking the time to work on this.
>>
>> "Siavosh Benabbas" <sbenabas at gmail.com> wrote:
>> > Thanks for the advice.
>> > I have cloned my version of git and commited the patch there. It is at
>> > http://www.cs.toronto.edu/~siavosh/parted-freebsd/
>> > Unfortunately directory listing is disabled on the server I don't know
>> > if I need to do anything special to make this into a git repository
>> > but I have placed a compressed version of the directory here:
>> > http://www.cs.toronto.edu/~siavosh/parted-freebsd.tar.gz
>> > I have removed the _GNU_SOURCE from the patch but can't compile here
>> > as I don't have gettext>=1.15.
>>
>> Can't you just get and install the latest version of gettext?
> Well it is a bit harder than that. A lot of things depend on gettext
> and the latest version has not been added to FreeBSD's port
> collection, I would like to avoid a build outside of that collection.
> I will do this sometime soon and let you know.

Note that the latest stable release is gettext-1.16.1 (oct 2006).

>> > The patch is attached.
>>
>> > diff --exclude=.git -NruBb parted/configure.ac parted-freebsd/configure.ac
>> > --- parted/configure.ac       Tue Feb 27 21:12:25 2007
>> > +++ parted-freebsd/configure.ac       Tue Feb 27 21:22:01 2007
>> > @@ -51,6 +51,8 @@
>> >       linux*) OS=linux ;;
>> >       gnu*)   OS=gnu ;;
>> >       beos*)  OS=beos ;;
>> > +     freebsd* | kfreebsd*-gnu)
>> > +                     OS=freebsd ;;
>> >       *)      AC_MSG_ERROR([Unknown or unsupported OS "$host_os".  Only "linux", "gnu" and "beos" are supported in this version of GNU Parted.]) ;;
>> >  esac
>> >  AC_SUBST(OS)
>> > @@ -184,6 +186,7 @@
>> >       #include <sys/types.h>
>> >       #include <unistd.h>
>> >  ])
>> > +AC_CHECK_TYPE(loff_t, long long)
>>
>> Why do you want to define the loff_t type at this level?
>> Won't it cause conflicts with the existing definitions?
>>
>>     libparted/fs/ext2/ext2.h:48:  typedef off_t loff_t;
>>     libparted/fs/xfs/platform_defs.h:61:typedef loff_t xfs_off_t;
>>
>> What happens if it is not defined?
>> Can you fix it with a less-global change?
>>
>> --------------------------
> Aren't these filesystem specific headers? I think we need such a
> definition for FreeBSD. I will give that a try.

Hm... I think a cleaner solution might be to simply remove the
typedef from ext2.h.  I've just noticed that the other file merely
*uses* "loff_t", it doesn't define it.

Then add this line in configure.ac:
[note use of "off_t", not "long long"]

  AC_CHECK_TYPE(loff_t, off_t)

> Well FreeBSD does provide sigaction but it misses (except for the
> CURRENT (unstable) branch) many MACRO constants that you can check the
> value of siginfo_t variables against. But I think that change in the
> configure.ac is avoidable.
>
>> The following change looks avoidable.
>> There should be a move to *remove* casts, not to add them,
>> especially, ones that look like that :)
>> I'll bet we can find a better way.
>>
>> > -    signal (SIGFPE, &sigfpe_handler);
>> > +    signal (SIGFPE, (void (*) (int)) &sigfpe_handler);
>>
>
> I agree that there should be a move to remove the casts but only by
> making the argument of correct type. Here we are using one
> sigfpe_handler function for two cases:
> 1. The system has sigaction which takes a pointer to a function with
> three arguments.
> 2. The system has signal which takes a pointer to a function with one arguments.

If you must use the cast, at least use the more portable "sighandler_t".

Also be careful when preparing a change.
If you change a file with only-spaces being used for
indentation, yet you insert lines with TABs for indentation,
the resulting patch can be very hard to read -- it can even be
seriously misleading.

So, please try to preserve the indentation type.
Yes, I know it's a pain, because parted is not consistent in this.
That is something that would be good to change, but it's also
very invasive...