1:57 p.m.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
src/libvirt-storage.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 48996ba..c4f2a03 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -233,7 +233,8 @@ virConnectNumOfDefinedStoragePools(virConnectPtr conn)
virCheckConnectReturn(conn, -1);
- if (conn->storageDriver &&
conn->storageDriver->connectNumOfDefinedStoragePools) {
+ if (conn->storageDriver &&
+ conn->storageDriver->connectNumOfDefinedStoragePools) {
int ret;
ret = conn->storageDriver->connectNumOfDefinedStoragePools(conn);
if (ret < 0)
@@ -280,7 +281,8 @@ virConnectListDefinedStoragePools(virConnectPtr conn,
virCheckNonNullArgGoto(names, error);
virCheckNonNegativeArgGoto(maxnames, error);
- if (conn->storageDriver &&
conn->storageDriver->connectListDefinedStoragePools) {
+ if (conn->storageDriver &&
+ conn->storageDriver->connectListDefinedStoragePools) {
int ret;
ret = conn->storageDriver->connectListDefinedStoragePools(conn, names,
maxnames);
if (ret < 0)
@@ -332,7 +334,8 @@ virConnectFindStoragePoolSources(virConnectPtr conn,
virCheckNonNullArgGoto(type, error);
virCheckReadOnlyGoto(conn->flags, error);
- if (conn->storageDriver &&
conn->storageDriver->connectFindStoragePoolSources) {
+ if (conn->storageDriver &&
+ conn->storageDriver->connectFindStoragePoolSources) {
char *ret;
ret = conn->storageDriver->connectFindStoragePoolSources(conn, type,
srcSpec, flags);
if (!ret)
@@ -485,7 +488,8 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
virCheckStorageVolReturn(vol, NULL);
- if (vol->conn->storageDriver &&
vol->conn->storageDriver->storagePoolLookupByVolume) {
+ if (vol->conn->storageDriver &&
+ vol->conn->storageDriver->storagePoolLookupByVolume) {
virStoragePoolPtr ret;
ret = vol->conn->storageDriver->storagePoolLookupByVolume(vol);
if (!ret)
@@ -1188,7 +1192,8 @@ virStoragePoolNumOfVolumes(virStoragePoolPtr pool)
virCheckStoragePoolReturn(pool, -1);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolNumOfVolumes) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storagePoolNumOfVolumes) {
int ret;
ret = pool->conn->storageDriver->storagePoolNumOfVolumes(pool);
if (ret < 0)
@@ -1230,7 +1235,8 @@ virStoragePoolListVolumes(virStoragePoolPtr pool,
virCheckNonNullArgGoto(names, error);
virCheckNonNegativeArgGoto(maxnames, error);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolListVolumes) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storagePoolListVolumes) {
int ret;
ret = pool->conn->storageDriver->storagePoolListVolumes(pool, names,
maxnames);
if (ret < 0)
@@ -1297,7 +1303,8 @@ virStorageVolLookupByName(virStoragePoolPtr pool,
virCheckStoragePoolReturn(pool, NULL);
virCheckNonNullArgGoto(name, error);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storageVolLookupByName) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storageVolLookupByName) {
virStorageVolPtr ret;
ret = pool->conn->storageDriver->storageVolLookupByName(pool, name);
if (!ret)
@@ -1471,7 +1478,8 @@ virStorageVolCreateXML(virStoragePoolPtr pool,
virCheckNonNullArgGoto(xmlDesc, error);
virCheckReadOnlyGoto(pool->conn->flags, error);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storageVolCreateXML) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storageVolCreateXML) {
virStorageVolPtr ret;
ret = pool->conn->storageDriver->storageVolCreateXML(pool, xmlDesc,
flags);
if (!ret)
--
2.1.0
4:04 a.m.
On Wed, Sep 28, 2016 at 12:27:21AM +0530, Nitesh Konkar wrote:
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
src/libvirt-storage.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 48996ba..c4f2a03 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -233,7 +233,8 @@ virConnectNumOfDefinedStoragePools(virConnectPtr conn)
virCheckConnectReturn(conn, -1);
- if (conn->storageDriver &&
conn->storageDriver->connectNumOfDefinedStoragePools) {
+ if (conn->storageDriver &&
+ conn->storageDriver->connectNumOfDefinedStoragePools) {
int ret;
ret = conn->storageDriver->connectNumOfDefinedStoragePools(conn);
if (ret < 0)
@@ -280,7 +281,8 @@ virConnectListDefinedStoragePools(virConnectPtr conn,
virCheckNonNullArgGoto(names, error);
virCheckNonNegativeArgGoto(maxnames, error);
- if (conn->storageDriver &&
conn->storageDriver->connectListDefinedStoragePools) {
+ if (conn->storageDriver &&
+ conn->storageDriver->connectListDefinedStoragePools) {
int ret;
ret = conn->storageDriver->connectListDefinedStoragePools(conn, names,
maxnames);
if (ret < 0)
@@ -332,7 +334,8 @@ virConnectFindStoragePoolSources(virConnectPtr conn,
virCheckNonNullArgGoto(type, error);
virCheckReadOnlyGoto(conn->flags, error);
- if (conn->storageDriver &&
conn->storageDriver->connectFindStoragePoolSources) {
+ if (conn->storageDriver &&
+ conn->storageDriver->connectFindStoragePoolSources) {
char *ret;
ret = conn->storageDriver->connectFindStoragePoolSources(conn, type,
srcSpec, flags);
if (!ret)
@@ -485,7 +488,8 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
virCheckStorageVolReturn(vol, NULL);
- if (vol->conn->storageDriver &&
vol->conn->storageDriver->storagePoolLookupByVolume) {
+ if (vol->conn->storageDriver &&
+ vol->conn->storageDriver->storagePoolLookupByVolume) {
virStoragePoolPtr ret;
ret = vol->conn->storageDriver->storagePoolLookupByVolume(vol);
if (!ret)
@@ -1188,7 +1192,8 @@ virStoragePoolNumOfVolumes(virStoragePoolPtr pool)
virCheckStoragePoolReturn(pool, -1);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolNumOfVolumes) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storagePoolNumOfVolumes) {
int ret;
ret = pool->conn->storageDriver->storagePoolNumOfVolumes(pool);
if (ret < 0)
@@ -1230,7 +1235,8 @@ virStoragePoolListVolumes(virStoragePoolPtr pool,
virCheckNonNullArgGoto(names, error);
virCheckNonNegativeArgGoto(maxnames, error);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolListVolumes) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storagePoolListVolumes) {
int ret;
ret = pool->conn->storageDriver->storagePoolListVolumes(pool, names,
maxnames);
if (ret < 0)
@@ -1297,7 +1303,8 @@ virStorageVolLookupByName(virStoragePoolPtr pool,
virCheckStoragePoolReturn(pool, NULL);
virCheckNonNullArgGoto(name, error);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storageVolLookupByName) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storageVolLookupByName) {
virStorageVolPtr ret;
ret = pool->conn->storageDriver->storageVolLookupByName(pool, name);
if (!ret)
@@ -1471,7 +1478,8 @@ virStorageVolCreateXML(virStoragePoolPtr pool,
virCheckNonNullArgGoto(xmlDesc, error);
virCheckReadOnlyGoto(pool->conn->flags, error);
- if (pool->conn->storageDriver &&
pool->conn->storageDriver->storageVolCreateXML) {
+ if (pool->conn->storageDriver &&
+ pool->conn->storageDriver->storageVolCreateXML) {
virStorageVolPtr ret;
ret = pool->conn->storageDriver->storageVolCreateXML(pool, xmlDesc,
flags);
if (!ret)
NACK this kind of change is just pointless. Further you've only line
wrapped half of the long lines in this patch context. We do *not* apply
a strict 80 character limit in libvirt.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
4:22 a.m.
On Wed, 2016-09-28 at 10:04 +0100, Daniel P. Berrange wrote:
We do *not* apply
a strict 80 character limit in libvirt.
Is that so? I was under the impression that we did, mostly
due to the fact that long lines are often pointed out
during review, but turns out that in fact the HACKING file
mentions no such rule.
There are some hints of encouraging, if not enforcing,
lines that do not exceed 80 columns, like
if (expr)
die("a diagnostic that would make this line"
" extend past the 80-column limit"));
or the sample invocation of indent(1) using the '-l75
-lc75' arguments. Plus the prohibit_long_lines rule that's
enforced as part of syntax-check
# Long lines can be harder to diff; too long, and
# git send-email chokes.
# For now, only enforce line length on files where
# we have intentionally
# fixed things and don't want to regress.
sc_prohibit_long_lines:
(a)prohibit='.{90}' \
in_vc_files='\.arg[sv]' \
halt='Wrap long lines in expected output files' \
$(_sc_search_regexp)
(a)prohibit='.{80}' \
in_vc_files='Makefile\.am' \
halt='Wrap long lines in Makefiles' \
$(_sc_search_regexp)
but is limited to a small number of non-C files.
Personally, I find the 80-column limit a pointless vestige
of long gone days; moreover, trying to enforce that limit
leads to code that is noticeably harder to read and wastes
more than 50% of the typical developer's monitor width.
Can we agree on something more reasonable, like 100 columns,
and document it / try to actually enforce it? From a cursory
look, it seems like we're 90% of the way there anyway.
--
Andrea Bolognani / Red Hat / Virtualization
3:28 a.m.
Hello Daniel,
Thanks for the feedback . Actually I only line wrapped those lines
exceeding
80 column limit. Rest I left unchanged. I particularly sent this patch to
get
feedback on whether its a rule or not as HACKING file hints at it
and patch reviews point at it.
Anywaz it does not make any sense now as its clear that we do not apply it
strictly.
Would it be wise to document this in hacking.html saying we encourage but
not
enforce it?
Thanks,
Nitesh.
On Wed, Sep 28, 2016 at 2:34 PM, Daniel P. Berrange <berrange(a)redhat.com>
wrote:
On Wed, Sep 28, 2016 at 12:27:21AM +0530, Nitesh Konkar wrote:
> Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
> ---
> src/libvirt-storage.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
> index 48996ba..c4f2a03 100644
> --- a/src/libvirt-storage.c
> +++ b/src/libvirt-storage.c
> @@ -233,7 +233,8 @@ virConnectNumOfDefinedStoragePools(virConnectPtr
conn)
>
> virCheckConnectReturn(conn, -1);
>
> - if (conn->storageDriver && conn->storageDriver->
connectNumOfDefinedStoragePools) {
> + if (conn->storageDriver &&
> + conn->storageDriver->connectNumOfDefinedStoragePools) {
> int ret;
> ret = conn->storageDriver->connectNumOfDefinedStoragePool
s(conn);
> if (ret < 0)
> @@ -280,7 +281,8 @@ virConnectListDefinedStoragePools(virConnectPtr
conn,
> virCheckNonNullArgGoto(names, error);
> virCheckNonNegativeArgGoto(maxnames, error);
>
> - if (conn->storageDriver && conn->storageDriver->
connectListDefinedStoragePools) {
> + if (conn->storageDriver &&
> + conn->storageDriver->connectListDefinedStoragePools) {
> int ret;
> ret = conn->storageDriver->connectListDefinedStoragePools(conn,
names, maxnames);
> if (ret < 0)
> @@ -332,7 +334,8 @@ virConnectFindStoragePoolSources(virConnectPtr conn,
> virCheckNonNullArgGoto(type, error);
> virCheckReadOnlyGoto(conn->flags, error);
>
> - if (conn->storageDriver &&
conn->storageDriver->connectFindStoragePoolSources)
{
> + if (conn->storageDriver &&
> + conn->storageDriver->connectFindStoragePoolSources) {
> char *ret;
> ret = conn->storageDriver->connectFindStoragePoolSources(conn,
type, srcSpec, flags);
> if (!ret)
> @@ -485,7 +488,8 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
>
> virCheckStorageVolReturn(vol, NULL);
>
> - if (vol->conn->storageDriver &&
vol->conn->storageDriver->storagePoolLookupByVolume)
{
> + if (vol->conn->storageDriver &&
> + vol->conn->storageDriver->storagePoolLookupByVolume) {
> virStoragePoolPtr ret;
> ret = vol->conn->storageDriver->storagePoolLookupByVolume(vol);
> if (!ret)
> @@ -1188,7 +1192,8 @@ virStoragePoolNumOfVolumes(virStoragePoolPtr pool)
>
> virCheckStoragePoolReturn(pool, -1);
>
> - if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolNumOfVolumes)
{
> + if (pool->conn->storageDriver &&
> + pool->conn->storageDriver->storagePoolNumOfVolumes) {
> int ret;
> ret = pool->conn->storageDriver->storagePoolNumOfVolumes(pool);
> if (ret < 0)
> @@ -1230,7 +1235,8 @@ virStoragePoolListVolumes(virStoragePoolPtr pool,
> virCheckNonNullArgGoto(names, error);
> virCheckNonNegativeArgGoto(maxnames, error);
>
> - if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolListVolumes)
{
> + if (pool->conn->storageDriver &&
> + pool->conn->storageDriver->storagePoolListVolumes) {
> int ret;
> ret = pool->conn->storageDriver->storagePoolListVolumes(pool,
names, maxnames);
> if (ret < 0)
> @@ -1297,7 +1303,8 @@ virStorageVolLookupByName(virStoragePoolPtr pool,
> virCheckStoragePoolReturn(pool, NULL);
> virCheckNonNullArgGoto(name, error);
>
> - if (pool->conn->storageDriver &&
pool->conn->storageDriver->storageVolLookupByName)
{
> + if (pool->conn->storageDriver &&
> + pool->conn->storageDriver->storageVolLookupByName) {
> virStorageVolPtr ret;
> ret = pool->conn->storageDriver->storageVolLookupByName(pool,
name);
> if (!ret)
> @@ -1471,7 +1478,8 @@ virStorageVolCreateXML(virStoragePoolPtr pool,
> virCheckNonNullArgGoto(xmlDesc, error);
> virCheckReadOnlyGoto(pool->conn->flags, error);
>
> - if (pool->conn->storageDriver &&
pool->conn->storageDriver->storageVolCreateXML)
{
> + if (pool->conn->storageDriver &&
> + pool->conn->storageDriver->storageVolCreateXML) {
> virStorageVolPtr ret;
> ret = pool->conn->storageDriver->storageVolCreateXML(pool,
xmlDesc, flags);
> if (!ret)
NACK this kind of change is just pointless. Further you've only line
wrapped half of the long lines in this patch context. We do *not* apply
a strict 80 character limit in libvirt.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/
:|
|: http://libvirt.org -o- http://virt-manager.org
:|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/
:|
2967
days inactive
2981
days old
3 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrange -
Nitesh Konkar