Hi, Bruno and others.

On Sep 07 2009, Bruno Cornec wrote:
> Rogério Brito said on Fri, Sep 04, 2009 at 01:07:04PM -0300:
> > Please, just wait a little bit. It would be nice to have a sort-of
> > peer-review on the code.
> 
> If you want.

I will do that asap.

> Well, I prefer to give you immediately my feeling on that. 
> 
> 1/ I didn't write that code. Hugo Rabson did in 2000. And that code is
> far from being at the level of quality I'd like it to be. So I agree
> that changes are needed to improve stuff.

Very nice. We can start cleaning that in the near future, so.

> 2/ We need to change but maintain our level of robustness after all. If
> you look at 2.2.10, (which is not workign atm) you'll see it's far from
> being easy.

Working reliably and dealing with error conditions is something that is
quite hard indeed.

> 3/ My background is 20 years of C/Unix, 16 of Linux, 10 of perl, ...
> with both professional and personal coding.

Mine is not much different from that (perhaps a little smaller than you,
with only 15 years of Unix and Linux experience).

> I'm not a pure expert, but know where I want the code to go. And my
> will is:
>   a/ remove useless code,

This is essential. Getting things smaller is always better.

>      useless static memory allocation (tricky)

I'm not sure about which static memory allocation you mean. Do you mean
"char foo[BIG_SIZE_HERE]"? Depending on the situation, just including
some malloc's/free's are gratuitous artificial complexity.

And, anyway, if said code is not used in a non-standard ways, it is
marked "auto" by the compiler and disposed when the scope of the
function ends.

>   b/ use valgrind and the like to improve quality + adhoc scripts.

Absolutely agreed, as I have pointed out in my earlier e-mail.

>   c/ in the long term add new features as perl code to make it much more
>      easy to deal with: doing string manipulation in C is just crazy,
>      and error prone.

I usually favor statically compiled languages, but I'm not opposed to
Perl, *if* the programs don't inflate the memory requirements of the
interpreter too much.

We would like to want the code to work even on small platforms, after
all.

> > I'm not sure exactly which changes you have already implemented. It  
> > seems that the code is hosted in a SVN repo, is that correct?
> 
> yes: http://trac.mondorescue.org/browser/branches

Thanks. I'll check that out right now.

> Could you send them to me for review, and discussion ?

Sure, but I'd guess that they may not be that interesting, now that the
versions have diverged a bit.

I'm including it here anyway. Please, note that the patch has already
three patches in it (patch in patch format, to be handled with quilt).

> Ah ! That's a good discussion. I'm living in a world where I meet a lot
> of conservative people in term of OS selection.

OK, I don't have any problems with that. Maintaining support for those
versions should not be that hard. Actually, almost nothing should be
needed.

> I also know that backports are working for Debian, so that could also
> well be interesting.

You should consider that.

> > This is a good thing. I will check it out and compare incremental  
> > changes from 2.2.7 -> 2.2.8 -> 2.2.9.
> >
> > Have you changed things a lot?
> 
> Well:
> 2.2.7 -> 2.2.8: 
> http://trac.mondorescue.org/changeset?new=branches%2F2.2.8%402259&old=branches%2F2.2.7%402045
(...)

A fair amount of changes to review. (Some small typos in the
documentation, though, but there's still some things to read).

> If we can avoid duplication of effort it's great.

Considering that the workforce is so small, it would be better to
concentrate it.

> Now as long as there are differences between our content, keeping it
> in 2 places is not a problem either, especially if one can be
> generated from the other.

I don't really see that need for the moment. Even if the trees diverged,
it would still be a good thing to use a single repository and create
branches (and merge them, when appropriate).

> I think we will find agreements ;-)

Indeed. :-)

> >  * include support for PowerPC (at least, NewWorld powermacs), Debian 
> > kFreeBSD i386, and kFreeBSD amd64.  This should be a little easier now, 
> > since we are soon getting to use GRUB 2, which is going to deprecate 
> > GRUB-legacy in the near future and provide a uniform platform for booting 
> > with many arches.
> 
> Not done. Remember, I like to keep support for multiple
> distro/arches/versions durnig time. So that support should be added, and
> we still need to keep LILO/GRUB/ELILO/...

No problems with that. For the moment, GRUB2 is still a WIP and I would
like to support yaboot for powerpc. I'm not sure what is the boot
methods of kFreeBSD are yet (still learning about the arches).

> >  * support lzma as a compression method and, in fact, generalize so that 
> > we can allow the user some arbitrary compression methods.
> 
> Done in 2.2.10 for lzma.

Very nice. I am short on space and like to keep my backups under 500MB,
so that I can use dvdisaster (yes, I am a bit paranoid).

BTW, by using dvdisaster, one can create solid archives (like I
mentioned with you many moons ago). And the backups are smaller
themselves.

And the extra redundancy really helps regarding getting things out of
defective media.

> Generalization was for me a topic of 3.x.x.

OK, in a slightly tangent comment, it seems that the version 2.2.7 has
problems with Linux images that have not been compressed with gzip
(newer kernels allow bzip2 and lzma compressed images). Do you have
support for that?

Right now, mondo dies trying to be smart about the kernel that I
compiled if I use one of those images, but it is wrong in its decision.

> >  * include support for burning CDs with dao instead of the tao method 
> > (this is a bug that I filed on Debian quite some time ago).
> 
> Yes. Should not be too hard.

Should be quite easy, depending on the situation. And, BTW, why are
there differences regarding burning CDs/CD-RWs? I can't really see the
point (besides the fact that CD-RWs may have to be blanked before
writing them).

> >  * abstract the way that support for initramfs/initrd is checked, as
> > not all kernels can be verified that way. Ultimately, trust the
> > user, following the good old tradition of giving him enough rope to
> > hang himself.
> 
> Well, here we need both. Good defaults, good detection + manual
> overwrite.

I absolutely agree that sane defaults are essential. But letting the
experienced user override the defaults is important to have a good share
of the target "market" for this tool.

The system administrators are the first to be worried about backups
after all.

(This doesn't mean that mondo should not be made easy to be used as,
say, a primary choice of backup for Desktop users, even if someone
latter, provides a fancy GUI for handholding said user).

> Which is for me a global rule as much as possible. Now it wasn't
> Hugo's rule, so it's sometimes difficult to implement.

Morphing the code can be done in steps, I think.

> >  * refactor the code so that we get less code duplication.
> 
> Agree.
> 
> >  * refactor the code so that we can include more things in the libmondo 
> > thing, proper for abstracting things that would be better adapted for new 
> > platforms.
> 
> Well I also agree. However, time for a rewrite is not far now. So I
> think we should keep those principle for the newer perl based version.

No problems with postponing things. But I am much more favorable to
gradual changes than to radical changes.

Something like what the kernel does: they are including newer features
and, while they are doing that, when they see something that is wrong,
they modify it.

Over-engineering is a problem that should be avoided. A certain degree
of generality is good, but not that much.

> Done. I run it regularly on 2.2.8, 2.2.9, 2.2.10. However, you test only
> the branch int the code that you use. And there are so many branches :-(

Great. Now that GCC 4.4 has test coverage, we can see which paths are
used and which ones are not.

> >  * run static analysis tools such as splint on the source code.
> 
> Needs customization. Look at tools/quality.

There are things with more priority than this at the moment. Getting
mondo to correctly backup some of my systems is more important than
having the code shining. Correctness in the first place.

(Actually, correctness will bring elegance with it).

> >  * compile the code with -Wall -Wextra -Werror -Os -g and fix the  
> > warnings. Making all warnings explicit usually exposes bad coding  
> > practices.
> 
> Improved a lot in 2.2.10 by bringing low level functions and right
> prototype. -Werror is not realistic as of today.

No problems.

> >  * get busybox-mind merged upstream as much as possible (ideally, we  
> > would not have to have busybox-mindi used by the mondo/mindi).
> 
> One of my ideas was to remove mindi-busybox alltogether.

Amem to that.

> Size is less a constraint today as it was in 2000 when the project
> started.

Size is still important (and will always be), but 1MB of a 650MB CD
wouldn't make that much difference.

> So just using the native tools instead of busybox should be the way to
> go. Cause only issues for dedicated tools (dhcp request e.g.)

Let's postpone this for further analysis.

> >  * audit the code so that suspect things can be eliminated (say, magic 
> > constants should have a name stating their purpose).
> 
> Agreed. In process on 2.2.10.

Great.

> Cf:
> http://sourceforge.net/mailarchive/message.php?msg_name=20090821144730.GE28687%40morley.gre.hp.com

I will check that as soon as I'm done writing this mail.

> >  * cleaning the logs a little bit.
> 
> You mean the project logs or Debian logs ?

The logs generated by mondo.

Some sentences could be shortened and replaced by more precise
statements of what is being done. Some formatting should be done to make
them easier to read (and to parse with automatic tools).

> >  * write the documentation in a format that is less painful than plain 
> > *roff. My proposal: generate the manpages from Perl's POD, as it is 
> > easier to understand since it is closer to plain text than the *roff 
> > formats).
> 
> yes. Once we have a perl project, using pod is just the way to go ;-)

Agreed.

> >  * take care of the coding standards.
> 
> Well, mine are the best, you should know :-)

Of course, as mine are. :-)

> As I'm mostly alone doing it currently, I geenraly modify code as time
> pass the way I want it to be. (I don't care about 80 col e.g., but care
> about lines, tabs, ...).

I'm religious about the 80 columns limit, but I have been known to be an
heretic from time to time. :-)

> Then for variable/functions names it's now a
> bit late, and would generate lots of work for less return.

Hummm, variable and function names is a hard problem (especially
guaranteeing that they don't collide), but this is also on my list of
things to change.

> THat's a good point and is easy to do. Indeed I could make a 2.2.9.1
> after 2.2.9 if pbs are found. A la kernel.org. Good point. I like that.

Nice. The key thing here is "be conservative". Even other major projects
do it that way (say, Mozilla).

> >  * make the Debian package depend on less tools and recommend them  
> > instead (APT/aptitude already install recommends automatically, but the 
> > experienced system administrator should have control of what gets  
> > installed on his system).
> 
> You're the best person to tell me what to do here.

Nice.

> I just think it should also be aligned with what we have on other distro
> as much as possible. I also consider having more packages to manage
> correctly deps.

The changes that I'm thinking about are to change from "very hard
dependencies to hard dependencies, which the package manager will still
obey, but that can be overridden by the administrator, as he is the one
that knows best what he needs".

For the regular users, no changes will be seen.

> > Sure, it is a highly useful (pair of) program(s). And has saved me
> > once, due to my daily backup paranoia. :-)
> 
> Great ! And I know that it saved a system managed by Bdale Garbee last
> year.

This seems to be a really small world, as I work with Bdale on another
project. :-)


Regards, Rogério Brito.

-- 
Rogério Brito : rbr...@{mackenzie,ime.usp}.br : GPG key 1024D/7C2CAEB8
http://www.ime.usp.br/~rbrito : http://meusite.mackenzie.com.br/rbrito
Projects: algorithms.berlios.de : lame.sf.net : vrms.alioth.debian.org

Attachment: filtered-update.patch.lzma
Description: Binary data

Reply via email to