linux-kernel - Re: [PATCH 0/10] ide: flags query macros

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090223082539.GA24465@gollum.tnic>
Date:	Mon, 23 Feb 2009 09:25:39 +0100
From:	Borislav Petkov <petkovbb@...glemail.com>
To:	Sam Ravnborg <sam@...nborg.org>
Cc:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>,
	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/10] ide: flags query macros

On Mon, Feb 23, 2009 at 08:34:52AM +0100, Sam Ravnborg wrote:
> On Mon, Feb 23, 2009 at 08:04:13AM +0100, Borislav Petkov wrote:
> > > Since it is not a lot of modified lines and the change is rather
> > > straightforward it could as well be done in a single patch...
> > 
> > here's version 2:
> > --
> > From: Borislav Petkov <petkovbb@...il.com>
> > Date: Mon, 23 Feb 2009 07:58:23 +0100
> > Subject: [PATCH] ide: flags query macros
> > 
> > Add drive->atapi_flags and drive->dev_flags macro wrappers
> > 
> > v2:
> > - use static inlines for better typechecking
> > - use macro indirection for convenience
> > 
> > Signed-off-by: Borislav Petkov <petkovbb@...il.com>
> 
> Version 2 looks much better - thanks.
> 
> diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
> index df3df00..3553759 100644
> --- a/drivers/ide/ide-cd_ioctl.c
> +++ b/drivers/ide/ide-cd_ioctl.c
> @@ -86,7 +86,7 @@ int ide_cdrom_check_media_change_real(struct cdrom_device_info *cdi,
> 
>         if (slot_nr == CDSL_CURRENT) {
>                 (void) cdrom_check_status(drive, NULL);
> -               retval = (drive->dev_flags & IDE_DFLAG_MEDIA_CHANGED) ? 1 : 0;
> +               retval = ide_dev_media_changed(drive) ? 1 : 0;
>                 drive->dev_flags &= ~IDE_DFLAG_MEDIA_CHANGED;
>                 return retval;
>         } else {
> 
> The use of ? 1 : 0; here is redundant.
> 
>                         if (drive->media == ide_disk) {
> -                               printk(KERN_INFO "%s: non-IDE drive, CHS=%d/%d/%d\n",
> +                               pr_info("%s: non-IDE drive, CHS=%d/%d/%d\n",
>                                         drive->name, drive->cyl,
>                                         drive->head, drive->sect);
>                         } else if (drive->media == ide_cdrom) {
> -                               printk(KERN_INFO "%s: ATAPI cdrom (?)\n", drive->name);
> +                               pr_info("%s: ATAPI cdrom (?)\n", drive->name);
>                         } else {
>                                 /* nuke it */
> -                               printk(KERN_WARNING "%s: Unknown device on bus refused identification. Ignoring.\n",
> +drive->name);
> +                               pr_warning("%s: Unknown device on bus refused "
> +                                          "identification, ignoring.\n",
> +                                          drive->name);
> I did not see this addressed in the changelog?

Actually, that was in the v1 changelog but got forgotten. Bart, can you
please add to the changelog

- shorten >80 lines

>                                 drive->dev_flags &= ~IDE_DFLAG_PRESENT;
> 
> You have a nice set of inlines to facilitate testing bits,
> but not for the above use?
> I guess this was not worth the abstraction for now.

Yeah, those are next but I'd like to wait a bit until ide rewrite
settles...

-- 
Regards/Gruss,
    Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/