On Sunday 27 January 2008, Borislav Petkov wrote:> From: Borislav Petkov <bbpetkov@yahoo.de>> > Some member names are self-explanatory, so remove their respective> comments. Also, explain the exact purpose of struct members in comments> in the struct definition instead of using excessively long member names> thus replacing then with a shorter, more handy version.> > Finally, remove unused members:

Even if this patch contains only trivial changes, the amount of themand the fact that it intermixes different logical changes (shorteningnames, dead code removal and comments beautification) makes it somehownon-trivial to review.

General comment:please have some mercy on the reviewer (in this case me ;) and spreadthe changes across more patches (it should also be easier for you sincewith more patches it is more likely that the more changes get appliedthe first time and you will have less code to recast/resubmit).

> diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c> index 4690f71..31edb0c 100644> --- a/drivers/ide/ide-tape.c> +++ b/drivers/ide/ide-tape.c> @@ -240,9 +240,9 @@ typedef struct idetape_stage_s {> } idetape_stage_t;> > /*> - * Most of our global data which we need to save even as we leave the> - * driver due to an interrupt or a timer event is stored in a variable> - * of type idetape_tape_t, defined below.> + * Most of our global data which we need to save even as we leave the driver due> + * to an interrupt or a timer event is stored in a variable of type> + * idetape_tape_t, defined below.

hmm, this comment looks ugly now because of an extra long first line...

> /* The first stage which will be removed from the pipeline */> idetape_stage_t *first_stage;> - /* The currently active stage */> idetape_stage_t *active_stage;> - /* Will be serviced after the currently active request */