* [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-23 10:02 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-23 10:02 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: kernel-janitors, linux-f2fs-devel
There is an error path where "dir" is an ERR_PTR.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 5148d90..921aede 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
out:
f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
"ino = %x, name = %s, dir = %lx, err = %d",
- ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
+ ino_of_node(ipage), raw_inode->i_name,
+ IS_ERR(dir) ? 0 : dir->i_ino, err);
return err;
}
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-23 10:02 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-23 10:02 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: kernel-janitors, linux-f2fs-devel
There is an error path where "dir" is an ERR_PTR.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 5148d90..921aede 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
out:
f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
"ino = %x, name = %s, dir = %lx, err = %d",
- ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
+ ino_of_node(ipage), raw_inode->i_name,
+ IS_ERR(dir) ? 0 : dir->i_ino, err);
return err;
}
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-23 10:02 ` [f2fs-dev] " Dan Carpenter
@ 2013-05-23 10:12 ` Dan Carpenter
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-23 10:12 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: kernel-janitors, linux-f2fs-devel
Btw, Linus's new Sparse changes don't like the f2fs's NEW_ADDR
macro.
#define NEW_ADDR -1U
We use it like this:
block_t src, dest;
if (src != dest && dest != NEW_ADDR && dest != NULL_ADDR) {
block_t is 64 bits so probably the macro should probably be:
#define NEW_ADDR ((block_t)-1)
I'm not able to test this so there may be some reason why changing
this breaks something. In that case we could do:
#define NEW_ADDR ((u32)(block_t)-1) /* explanation */
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-23 10:12 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-23 10:12 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: kernel-janitors, linux-f2fs-devel
Btw, Linus's new Sparse changes don't like the f2fs's NEW_ADDR
macro.
#define NEW_ADDR -1U
We use it like this:
block_t src, dest;
if (src != dest && dest != NEW_ADDR && dest != NULL_ADDR) {
block_t is 64 bits so probably the macro should probably be:
#define NEW_ADDR ((block_t)-1)
I'm not able to test this so there may be some reason why changing
this breaks something. In that case we could do:
#define NEW_ADDR ((u32)(block_t)-1) /* explanation */
regards,
dan carpenter
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-23 10:12 ` [f2fs-dev] " Dan Carpenter
@ 2013-05-24 3:46 ` Jaegeuk Kim
-1 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2013-05-24 3:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]
2013-05-23 (목), 13:12 +0300, Dan Carpenter:
> Btw, Linus's new Sparse changes don't like the f2fs's NEW_ADDR
> macro.
>
> #define NEW_ADDR -1U
>
> We use it like this:
>
> block_t src, dest;
>
> if (src != dest && dest != NEW_ADDR && dest != NULL_ADDR) {
>
> block_t is 64 bits so probably the macro should probably be:
>
> #define NEW_ADDR ((block_t)-1)
Hi,
Agreed that it needs to synchronize on-disk and in-memory block address
types, __le32 and u64.
How about the below patch?
>
> I'm not able to test this so there may be some reason why changing
> this breaks something. In that case we could do:
>
> #define NEW_ADDR ((u32)(block_t)-1) /* explanation */
>
> regards,
> dan carpenter
>
From ded3c78fcb6dd8c39a8664471544942e51ff9faf Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Date: Fri, 24 May 2013 12:41:04 +0900
Subject: [PATCH] f2fs: align data types between on-disk and in-memory
block
addresses
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
The on-disk block address is defined as __le32, but in-memory block
address,
block_t, does as u64.
Let's synchronize them to 32 bits.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
fs/f2fs/f2fs.h | 5 ++++-
include/linux/f2fs_fs.h | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 7b05029..92fd4e9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -37,7 +37,10 @@
typecheck(unsigned long long, b) && \
((long long)((a) - (b)) > 0))
-typedef u64 block_t;
+typedef u32 block_t; /*
+ * should not change u32, since it is the on-disk block
+ * address format, __le32.
+ */
typedef u32 nid_t;
struct f2fs_mount_info {
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index df6fab8..383d5e3 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -20,8 +20,8 @@
#define F2FS_BLKSIZE 4096 /* support only 4KB block */
#define F2FS_MAX_EXTENSION 64 /* # of extension entries */
-#define NULL_ADDR 0x0U
-#define NEW_ADDR -1U
+#define NULL_ADDR ((block_t)0) /* used as block_t addresses */
+#define NEW_ADDR ((block_t)-1) /* used as block_t addresses */
#define F2FS_ROOT_INO(sbi) (sbi->root_ino_num)
#define F2FS_NODE_INO(sbi) (sbi->node_ino_num)
--
1.8.1.3.566.gaa39828
--
Jaegeuk Kim
Samsung
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-24 3:46 ` Jaegeuk Kim
0 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2013-05-24 3:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
[-- Attachment #1.1: Type: text/plain, Size: 2587 bytes --]
2013-05-23 (목), 13:12 +0300, Dan Carpenter:
> Btw, Linus's new Sparse changes don't like the f2fs's NEW_ADDR
> macro.
>
> #define NEW_ADDR -1U
>
> We use it like this:
>
> block_t src, dest;
>
> if (src != dest && dest != NEW_ADDR && dest != NULL_ADDR) {
>
> block_t is 64 bits so probably the macro should probably be:
>
> #define NEW_ADDR ((block_t)-1)
Hi,
Agreed that it needs to synchronize on-disk and in-memory block address
types, __le32 and u64.
How about the below patch?
>
> I'm not able to test this so there may be some reason why changing
> this breaks something. In that case we could do:
>
> #define NEW_ADDR ((u32)(block_t)-1) /* explanation */
>
> regards,
> dan carpenter
>
From ded3c78fcb6dd8c39a8664471544942e51ff9faf Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
Date: Fri, 24 May 2013 12:41:04 +0900
Subject: [PATCH] f2fs: align data types between on-disk and in-memory
block
addresses
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net
The on-disk block address is defined as __le32, but in-memory block
address,
block_t, does as u64.
Let's synchronize them to 32 bits.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
---
fs/f2fs/f2fs.h | 5 ++++-
include/linux/f2fs_fs.h | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 7b05029..92fd4e9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -37,7 +37,10 @@
typecheck(unsigned long long, b) && \
((long long)((a) - (b)) > 0))
-typedef u64 block_t;
+typedef u32 block_t; /*
+ * should not change u32, since it is the on-disk block
+ * address format, __le32.
+ */
typedef u32 nid_t;
struct f2fs_mount_info {
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index df6fab8..383d5e3 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -20,8 +20,8 @@
#define F2FS_BLKSIZE 4096 /* support only 4KB block */
#define F2FS_MAX_EXTENSION 64 /* # of extension entries */
-#define NULL_ADDR 0x0U
-#define NEW_ADDR -1U
+#define NULL_ADDR ((block_t)0) /* used as block_t addresses */
+#define NEW_ADDR ((block_t)-1) /* used as block_t addresses */
#define F2FS_ROOT_INO(sbi) (sbi->root_ino_num)
#define F2FS_NODE_INO(sbi) (sbi->node_ino_num)
--
1.8.1.3.566.gaa39828
--
Jaegeuk Kim
Samsung
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 421 bytes --]
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
[-- Attachment #3: Type: text/plain, Size: 179 bytes --]
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-24 3:46 ` [f2fs-dev] " Jaegeuk Kim
@ 2013-05-24 11:23 ` Dan Carpenter
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-24 11:23 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: kernel-janitors, linux-f2fs-devel
On Fri, May 24, 2013 at 12:46:08PM +0900, Jaegeuk Kim wrote:
> From ded3c78fcb6dd8c39a8664471544942e51ff9faf Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> Date: Fri, 24 May 2013 12:41:04 +0900
> Subject: [PATCH] f2fs: align data types between on-disk and in-memory
> block
> addresses
> Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
> linux-f2fs-devel@lists.sourceforge.net
>
> The on-disk block address is defined as __le32, but in-memory block
> address,
> block_t, does as u64.
>
> Let's synchronize them to 32 bits.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
What you say sounds good to me. Could you give me the reported-by
tag instead of the Signed-off-by? The Signed-off-by means it went
through my hands and it's intended as sort of a legal thing like
signing a document.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-24 11:23 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-24 11:23 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: kernel-janitors, linux-f2fs-devel
On Fri, May 24, 2013 at 12:46:08PM +0900, Jaegeuk Kim wrote:
> From ded3c78fcb6dd8c39a8664471544942e51ff9faf Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> Date: Fri, 24 May 2013 12:41:04 +0900
> Subject: [PATCH] f2fs: align data types between on-disk and in-memory
> block
> addresses
> Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
> linux-f2fs-devel@lists.sourceforge.net
>
> The on-disk block address is defined as __le32, but in-memory block
> address,
> block_t, does as u64.
>
> Let's synchronize them to 32 bits.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
What you say sounds good to me. Could you give me the reported-by
tag instead of the Signed-off-by? The Signed-off-by means it went
through my hands and it's intended as sort of a legal thing like
signing a document.
regards,
dan carpenter
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-24 11:23 ` [f2fs-dev] " Dan Carpenter
@ 2013-05-24 11:32 ` Jaegeuk Kim
-1 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2013-05-24 11:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]
2013-05-24 (금), 14:23 +0300, Dan Carpenter:
> On Fri, May 24, 2013 at 12:46:08PM +0900, Jaegeuk Kim wrote:
> > From ded3c78fcb6dd8c39a8664471544942e51ff9faf Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> > Date: Fri, 24 May 2013 12:41:04 +0900
> > Subject: [PATCH] f2fs: align data types between on-disk and in-memory
> > block
> > addresses
> > Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
> > linux-f2fs-devel@lists.sourceforge.net
> >
> > The on-disk block address is defined as __le32, but in-memory block
> > address,
> > block_t, does as u64.
> >
> > Let's synchronize them to 32 bits.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
>
> What you say sounds good to me. Could you give me the reported-by
> tag instead of the Signed-off-by? The Signed-off-by means it went
> through my hands and it's intended as sort of a legal thing like
> signing a document.
Ok, I see.
Thank you very much for the notification. :)
>
> regards,
> dan carpenter
--
Jaegeuk Kim
Samsung
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-24 11:32 ` Jaegeuk Kim
0 siblings, 0 replies; 16+ messages in thread
From: Jaegeuk Kim @ 2013-05-24 11:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
[-- Attachment #1.1: Type: text/plain, Size: 1145 bytes --]
2013-05-24 (금), 14:23 +0300, Dan Carpenter:
> On Fri, May 24, 2013 at 12:46:08PM +0900, Jaegeuk Kim wrote:
> > From ded3c78fcb6dd8c39a8664471544942e51ff9faf Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> > Date: Fri, 24 May 2013 12:41:04 +0900
> > Subject: [PATCH] f2fs: align data types between on-disk and in-memory
> > block
> > addresses
> > Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
> > linux-f2fs-devel@lists.sourceforge.net
> >
> > The on-disk block address is defined as __le32, but in-memory block
> > address,
> > block_t, does as u64.
> >
> > Let's synchronize them to 32 bits.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
>
> What you say sounds good to me. Could you give me the reported-by
> tag instead of the Signed-off-by? The Signed-off-by means it went
> through my hands and it's intended as sort of a legal thing like
> signing a document.
Ok, I see.
Thank you very much for the notification. :)
>
> regards,
> dan carpenter
--
Jaegeuk Kim
Samsung
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 421 bytes --]
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
[-- Attachment #3: Type: text/plain, Size: 179 bytes --]
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-23 10:02 ` [f2fs-dev] " Dan Carpenter
@ 2013-05-26 15:05 ` walter harms
-1 siblings, 0 replies; 16+ messages in thread
From: walter harms @ 2013-05-26 15:05 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
Am 23.05.2013 12:02, schrieb Dan Carpenter:
> There is an error path where "dir" is an ERR_PTR.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 5148d90..921aede 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
> out:
> f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
> "ino = %x, name = %s, dir = %lx, err = %d",
> - ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
> + ino_of_node(ipage), raw_inode->i_name,
> + IS_ERR(dir) ? 0 : dir->i_ino, err);
> return err;
> }
>
I am not an expert on this matter so a simple question:
dir->i_ino=0 is not valid ?
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-26 15:05 ` walter harms
0 siblings, 0 replies; 16+ messages in thread
From: walter harms @ 2013-05-26 15:05 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
Am 23.05.2013 12:02, schrieb Dan Carpenter:
> There is an error path where "dir" is an ERR_PTR.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 5148d90..921aede 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
> out:
> f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
> "ino = %x, name = %s, dir = %lx, err = %d",
> - ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
> + ino_of_node(ipage), raw_inode->i_name,
> + IS_ERR(dir) ? 0 : dir->i_ino, err);
> return err;
> }
>
I am not an expert on this matter so a simple question:
dir->i_ino==0 is not valid ?
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-26 15:05 ` [f2fs-dev] " walter harms
@ 2013-05-26 20:41 ` Dan Carpenter
-1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-26 20:41 UTC (permalink / raw)
To: walter harms; +Cc: kernel-janitors, linux-f2fs-devel
On Sun, May 26, 2013 at 05:05:15PM +0200, walter harms wrote:
>
>
> Am 23.05.2013 12:02, schrieb Dan Carpenter:
> > There is an error path where "dir" is an ERR_PTR.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> > index 5148d90..921aede 100644
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
> > out:
> > f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
> > "ino = %x, name = %s, dir = %lx, err = %d",
> > - ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
> > + ino_of_node(ipage), raw_inode->i_name,
> > + IS_ERR(dir) ? 0 : dir->i_ino, err);
> > return err;
> > }
> >
> I am not an expert on this matter so a simple question:
> dir->i_ino=0 is not valid ?
That is a valid question. The trick is that we also print the err
code so error conditions should be pretty obvious.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-26 20:41 ` Dan Carpenter
0 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2013-05-26 20:41 UTC (permalink / raw)
To: walter harms; +Cc: kernel-janitors, linux-f2fs-devel
On Sun, May 26, 2013 at 05:05:15PM +0200, walter harms wrote:
>
>
> Am 23.05.2013 12:02, schrieb Dan Carpenter:
> > There is an error path where "dir" is an ERR_PTR.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> > index 5148d90..921aede 100644
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
> > out:
> > f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
> > "ino = %x, name = %s, dir = %lx, err = %d",
> > - ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
> > + ino_of_node(ipage), raw_inode->i_name,
> > + IS_ERR(dir) ? 0 : dir->i_ino, err);
> > return err;
> > }
> >
> I am not an expert on this matter so a simple question:
> dir->i_ino==0 is not valid ?
That is a valid question. The trick is that we also print the err
code so error conditions should be pretty obvious.
regards,
dan carpenter
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [patch] f2fs: dereferencing an ERR_PTR
2013-05-26 20:41 ` [f2fs-dev] " Dan Carpenter
@ 2013-05-27 7:13 ` walter harms
-1 siblings, 0 replies; 16+ messages in thread
From: walter harms @ 2013-05-27 7:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
Am 26.05.2013 22:41, schrieb Dan Carpenter:
> On Sun, May 26, 2013 at 05:05:15PM +0200, walter harms wrote:
>>
>>
>> Am 23.05.2013 12:02, schrieb Dan Carpenter:
>>> There is an error path where "dir" is an ERR_PTR.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>>> index 5148d90..921aede 100644
>>> --- a/fs/f2fs/recovery.c
>>> +++ b/fs/f2fs/recovery.c
>>> @@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
>>> out:
>>> f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
>>> "ino = %x, name = %s, dir = %lx, err = %d",
>>> - ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
>>> + ino_of_node(ipage), raw_inode->i_name,
>>> + IS_ERR(dir) ? 0 : dir->i_ino, err);
>>> return err;
>>> }
>>>
>> I am not an expert on this matter so a simple question:
>> dir->i_ino=0 is not valid ?
>
> That is a valid question. The trick is that we also print the err
> code so error conditions should be pretty obvious.
>
hi dan,
people never read error messages propperly, i would go for -1
or at least something obvoius like 57005 (0xdead :).
re,
wh
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [f2fs-dev] [patch] f2fs: dereferencing an ERR_PTR
@ 2013-05-27 7:13 ` walter harms
0 siblings, 0 replies; 16+ messages in thread
From: walter harms @ 2013-05-27 7:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kernel-janitors, linux-f2fs-devel
Am 26.05.2013 22:41, schrieb Dan Carpenter:
> On Sun, May 26, 2013 at 05:05:15PM +0200, walter harms wrote:
>>
>>
>> Am 23.05.2013 12:02, schrieb Dan Carpenter:
>>> There is an error path where "dir" is an ERR_PTR.
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>
>>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>>> index 5148d90..921aede 100644
>>> --- a/fs/f2fs/recovery.c
>>> +++ b/fs/f2fs/recovery.c
>>> @@ -71,7 +71,8 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
>>> out:
>>> f2fs_msg(inode->i_sb, KERN_NOTICE, "recover_inode and its dentry: "
>>> "ino = %x, name = %s, dir = %lx, err = %d",
>>> - ino_of_node(ipage), raw_inode->i_name, dir->i_ino, err);
>>> + ino_of_node(ipage), raw_inode->i_name,
>>> + IS_ERR(dir) ? 0 : dir->i_ino, err);
>>> return err;
>>> }
>>>
>> I am not an expert on this matter so a simple question:
>> dir->i_ino==0 is not valid ?
>
> That is a valid question. The trick is that we also print the err
> code so error conditions should be pretty obvious.
>
hi dan,
people never read error messages propperly, i would go for -1
or at least something obvoius like 57005 (0xdead :).
re,
wh
------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-05-27 7:13 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-23 10:02 [patch] f2fs: dereferencing an ERR_PTR Dan Carpenter
2013-05-23 10:02 ` [f2fs-dev] " Dan Carpenter
2013-05-23 10:12 ` Dan Carpenter
2013-05-23 10:12 ` [f2fs-dev] " Dan Carpenter
2013-05-24 3:46 ` Jaegeuk Kim
2013-05-24 3:46 ` [f2fs-dev] " Jaegeuk Kim
2013-05-24 11:23 ` Dan Carpenter
2013-05-24 11:23 ` [f2fs-dev] " Dan Carpenter
2013-05-24 11:32 ` Jaegeuk Kim
2013-05-24 11:32 ` [f2fs-dev] " Jaegeuk Kim
2013-05-26 15:05 ` walter harms
2013-05-26 15:05 ` [f2fs-dev] " walter harms
2013-05-26 20:41 ` Dan Carpenter
2013-05-26 20:41 ` [f2fs-dev] " Dan Carpenter
2013-05-27 7:13 ` walter harms
2013-05-27 7:13 ` [f2fs-dev] " walter harms
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.