[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: On gratuitous modularization
From: |
Vladimir 'φ-coder/phcoder' Serbinenko |
Subject: |
Re: On gratuitous modularization |
Date: |
Sun, 07 Feb 2010 01:37:35 +0100 |
User-agent: |
Mozilla-Thunderbird 2.0.0.22 (X11/20091109) |
Christian Franke wrote:
> Hi Robert,
>
> Robert Millan wrote:
>
>> Please be careful when adding modules. I see that too often new modules
>> are added without any real need to host this code separately.
>>
>> There are many examples where this happened. I just noticed:
>>
>> commands/hdparm.c: struct grub_disk_ata_pass_through_parms apt;
>> commands/hdparm.c: if (grub_disk_ata_pass_through (disk,&apt))
>> commands/hdparm.c: struct grub_disk_ata_pass_through_parms apt;
>> commands/hdparm.c: if (grub_disk_ata_pass_through (disk,&apt))
>> commands/hdparm.c: struct grub_disk_ata_pass_through_parms apt;
>> commands/hdparm.c: if (grub_disk_ata_pass_through (disk,&apt))
>> commands/hdparm.c: if (! grub_disk_ata_pass_through)
>> disk/ata_pthru.c: struct
>> grub_disk_ata_pass_through_parms *parms)
>> disk/ata_pthru.c: grub_disk_ata_pass_through = grub_ata_pass_through;
>> disk/ata_pthru.c: if (grub_disk_ata_pass_through ==
>> grub_ata_pass_through)
>> disk/ata_pthru.c: grub_disk_ata_pass_through = NULL;
>> include/grub/disk.h:struct grub_disk_ata_pass_through_parms
>> include/grub/disk.h:extern grub_err_t (*
>> EXPORT_VAR(grub_disk_ata_pass_through)) (grub_disk_t,
>> include/grub/disk.h: struct
>> grub_disk_ata_pass_through_parms *);
>> kern/disk.c:grub_err_t (* grub_disk_ata_pass_through) (grub_disk_t,
>> kern/disk.c: struct grub_disk_ata_pass_through_parms *);
>>
>> this seems unnecessary. ata_pthru is very small. If it's only used
>> by hdparm,
>> why not just merge it? This also avoids the additional code in kernel.
>>
>>
>
> The module ata_pthru.mod exists only to keep ata.mod small, see:
> http://lists.gnu.org/archive/html/grub-devel/2009-02/msg00091.html
>
> Keeping the ata.mod specific pass-through function separate from
> hdparm.mod was intentional. Merging this function into hdparm.mod
> would only make sense if ata.mod will the only ATA access module with
> pass-through functionality in the future. Hdparm.mod would then depend
> on ata.mod. A 'hdparm -h' would load ata.mod and disable biosdisk access.
>
> I hope we will eventually have an ahci.mod :-)
>
Are the parameters of current ata_pass_through ATA-specific or would
they be the same on AHCI?
> BTW: I agree that using a global function pointer
> 'grub_disk_ata_pass_through' is a hack. A cleaner design would be
> possible with a grub_disk_dev.ioctl(.) call.
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
- Re: On gratuitous modularization,
Vladimir 'φ-coder/phcoder' Serbinenko <=